Create a ComponentSource plugin for JS components

Created on 10 January 2025, 5 months ago

Overview

✨ [exploratory] PoC of Preact+Tailwind components editable via CodeMirror or Monaco Active has some proof-of-concept work for supporting JS components (initially just Astro islands implemented in Preact but this can be expanded over time), whose code can be edited within the XB UI, as components that can be used within XB just like Twig-based SDCs can be.

Per #3483267-29: [exploratory] PoC of Preact+Tailwind components editable via CodeMirror β†’ , we'll be opening a new Plan issue for turning that PoC into committable code in multiple granular steps. Once that Plan issue is opened, this issue will become one of those steps (child issues). However, this issue also is about code that has not yet been written in any form within that PoC, because we haven't gotten to it yet and it should just be straightforward Drupal code not requiring any new technologies to figure out.

Currently in that PoC, each JS component is exposed as an SDC and therefore requires a Twig file that renders the <astro-island> element. In this issue, we want to remove those Twig wrappers, so that when a new JS component is added, it just works, without needing Twig code to also be added.

Proposed resolution

XB already has two ComponentSource plugins: SingleDirectoryComponent and BlockComponent. Implement a 3rd one: JavaScriptComponent. Make this plugin, instead of a Twig file per component, render the <astro-island> element.

User interface changes

✨ Feature request
Status

Active

Version

0.0

Component

Theme builder

Created by

πŸ‡ΊπŸ‡ΈUnited States effulgentsia

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @effulgentsia
  • πŸ‡§πŸ‡ͺBelgium wim leers Ghent πŸ‡§πŸ‡ͺπŸ‡ͺπŸ‡Ί

    This should update the table at the top of 🌱 [META] Support component types other than SDC Needs work .

  • πŸ‡³πŸ‡±Netherlands balintbrews Amsterdam, NL
  • πŸ‡³πŸ‡±Netherlands balintbrews Amsterdam, NL
  • πŸ‡ΊπŸ‡ΈUnited States effulgentsia

    Up until now, the XB team has been following a pseudo-scrum/pseudo-kanban process, but we're now shifting into more conventional scrum. We started a new 2-week sprint last Thursday (Jan 16). I'm tagging our current sprint's issues for visibility.

  • πŸ‡§πŸ‡ͺBelgium wim leers Ghent πŸ‡§πŸ‡ͺπŸ‡ͺπŸ‡Ί
  • πŸ‡§πŸ‡ͺBelgium wim leers Ghent πŸ‡§πŸ‡ͺπŸ‡ͺπŸ‡Ί

    @balintbrews: Did you intend for this issue to deliver end-to-end working code components, in the sense that:

    • my code components appear in the left-hand in the XB UI
    • I can drag them onto the canvas and see a preview
    • I can populate their inputs using a field widget, and the preview updates
    • I can save it

    ?

  • πŸ‡³πŸ‡±Netherlands balintbrews Amsterdam, NL

    #7 ✨ Create a ComponentSource plugin for JS components Active : Yes to all of those, with the following notes.

    1. From the issue summary of ✨ Config entity for storing code components Active :

      The status property could potentially be used in ✨ Create a ComponentSource plugin for JS components Active to decide which code component to pick up and present as a component that can be used for creating content. Learn more about the distinction in ✨ Adding code components to components Active .

      I don't yet see that property added in the config entity. It's a different issue which is still in progress, but I wanted to highlight it, because it will be relevant here.

    2. Before we land ✨ Hydration library for code components Active , workarounds will be needed in order to see a preview of the components. From the issue summary of this issue:

      Follow the steps in this comment to attach the appropriate JS files that you need to manually place in your files folder: #3483267-17: [exploratory] PoC of Preact+Tailwind components editable via CodeMirror β†’ . We'll create a proper library to use in ✨ Hydration library for code components Active .

      If this issue lands before the hydration library, we'll adjust the component source plugin as part of the hydration library issue to remove the need for the workarounds.

  • πŸ‡§πŸ‡ͺBelgium wim leers Ghent πŸ‡§πŸ‡ͺπŸ‡ͺπŸ‡Ί

    Okay! That then shows that the

          // @todo Make actual render array.
          preview: [
            '#markup' => '@todo Make something πŸ†’',
          ],
    

    bit in ✨ Config entity for storing code components Active should indeed point here πŸ‘

    1. I don't yet see that property added in the config entity.

      It is β€” it's inherited from \Drupal\Core\Config\Entity\ConfigEntityBase::$status (and prescribed by type: config_entity in config schema).

    2. πŸ‘
  • πŸ‡¦πŸ‡ΊAustralia larowlan πŸ‡¦πŸ‡ΊπŸ.au GMT+10

    Updated issue summary to include items from a call with @effulgentsia

  • πŸ‡§πŸ‡ͺBelgium wim leers Ghent πŸ‡§πŸ‡ͺπŸ‡ͺπŸ‡Ί

    ✨ Config entity for storing code components Active is in! πŸ₯³

    Note: as described in the issue summary of #3499927 only if JavaScriptComponent::status() === TRUE should it get a Component config entity generated.

  • πŸ‡¦πŸ‡ΊAustralia larowlan πŸ‡¦πŸ‡ΊπŸ.au GMT+10
  • πŸ‡¦πŸ‡ΊAustralia larowlan πŸ‡¦πŸ‡ΊπŸ.au GMT+10

    Spent some time reviewing the original POC and how it wires up Astro
    I think there's probably scope to split out some smaller stories from this one and make this a parent story as follows:

    * Astro render element, this is reasonably straight forward in so far as it needs to take some #keys and render the twig like seen in the demo
    * A library alter hook that should dynamically build library definitions based on defined code component entities - we will probably need to think about dependencies between components when one component makes use of another, but we can equally punt that to a followup
    * Something similar to core's AssetControllerBase that serves CSS/JS from the assets:// stream wrapper to serve the compiled css and js as stored in the config entity. As mentioned we will likely need support for a unique hash of the contents and a preview flag. The preview flag will read from auto-saved entries. The hash will ensure we don't serve stale items. When we auto-generate libraries using the hook alter (item above) we will resolve these to the URLs for this controller. If the file exists on disk it will be served by apache. If it doesn't we'll write it to disk and then return it. This should be able to make use of a lot of the lazy asset work that landed in core in 10.1. The urls could be something like

        $directory_path = $this->streamWrapperManager->getViaScheme('assets')->getDirectoryPath();
        $routes['/' . $directory_path . '/xb-css/{js_component}/{hash}'] //...
    

    like we have in core in \Drupal\system\Routing\AssetRoutes::routes but rather than passing a filename in the route we could pass the machine name and hash. We can set the hash as the version in the library alter and use it in the library definition URLS.

    We can then append ?preview=1 to the URL for the preview version and likely add some access controls. At some point in the future this could use the workspace ID in the URL too, to allow discrete versions per workspace. The preview version most likely wouldn't dump to file as we would expect this to be regularly updated.

    For now I'll start on these in this story but it might be worth splitting them off into smaller issues if the MR size grows too large

  • Merge request !581Issue #3498889: Add component source plugin β†’ (Merged) created by larowlan
  • πŸ‡§πŸ‡ͺBelgium wim leers Ghent πŸ‡§πŸ‡ͺπŸ‡ͺπŸ‡Ί

    we will probably need to think about dependencies between components when one component makes use of another, but we can equally punt that to a followup

    +1, because that'd also require the config entity to know about those dependencies. That was not in scope for ✨ Config entity for storing code components Active (nor mentioned as future work, actually), and this is in the critical path for many other things. So let's stick to just individual code components in this issue πŸ™

    Something similar to core's AssetControllerBase that serves CSS/JS from the assets:// stream wrapper to serve the compiled css and js as stored in the config entity. […]

    I think it could be much simpler:

    β€” ✨ Storage for global CSS Active

    […] We can then append ?preview=1 to the URL for the preview version […]

    Why do we even need to load these assets while previewing? That'd require going from client to server and back to the client? The client-side already has literally all the information it needs (source+compiled CSS, source+compiled JS), I don't see why the server needs to be involved at all? πŸ€”

    Un-assigning @larowlan because it's a holiday in Australia on Monday. He'll likely be the one to push this forward further, but that shouldn't prevent somebody from doing so until he returns :)

  • πŸ‡³πŸ‡±Netherlands balintbrews Amsterdam, NL

    […] We can then append ?preview=1 to the URL for the preview version […]

    Why do we even need to load these assets while previewing? That'd require going from client to server and back to the client? The client-side already has literally all the information it needs (source+compiled CSS, source+compiled JS), I don't see why the server needs to be involved at all? πŸ€”

    I agree that we can keep the preview client-side. It's also how we implemented it in ✨ [exploratory] PoC of Preact+Tailwind components editable via CodeMirror or Monaco Active .

    In ✨ Preview for code components Active we need to do compilation with @swc/wasm-web and tailwindcss-in-browser in the browser anyway, at which point we have everything to display a preview. Like Wim said. 😊

  • First commit to issue fork.
  • πŸ‡¬πŸ‡§United Kingdom longwave UK

    Added the library hook and attached it to the island render array.

    Wondering whether, similar to core, the paths should be something like assets://xb/css/HASH.css?component=BLAH - this way if two components happen to share identical CSS they will share the same library definition as well. Core uses the query string to build and then the stored file doesn't otherwise care.

    Unless we don't need the preview at all as per #15/16.

  • πŸ‡¬πŸ‡§United Kingdom longwave UK

    Implemented asset generation as per #15.

  • πŸ‡¦πŸ‡ΊAustralia larowlan πŸ‡¦πŸ‡ΊπŸ.au GMT+10

    My thinking around the client side/server preview comes down to import maps. If we need an import maps it needs to be in an IFrame. But if it's already solved, that's awesome

  • πŸ‡§πŸ‡ͺBelgium wim leers Ghent πŸ‡§πŸ‡ͺπŸ‡ͺπŸ‡Ί

    Everything in this MR so far has focused on the astro island/rendering/assets aspects.

    But there's more to this issue:

    1. The issue summary states clearly that there must be a new ComponentSource plugin.
    2. I asked @balintbrews this in #7 and he responded affirmatively:

      my code components appear in the left-hand Library in the XB UI

    Working on those two aspects.

  • πŸ‡§πŸ‡ͺBelgium wim leers Ghent πŸ‡§πŸ‡ͺπŸ‡ͺπŸ‡Ί

    Delivered on much of #21, but not everthing yet. EOD, so passing context + next steps on to the next person:

    1. Managed to lift 600 LoC out of \Drupal\experience_builder\Plugin\ExperienceBuilder\ComponentSource\SingleDirectoryComponent into GeneratedFieldExplicitInputUxComponentSourceBase. I kept it as simple as possible, and resisted doing significant refactoring πŸ€“
    2. This made SingleDirectoryComponent merely ~275 LoC, and there's likely more still to be done.
    3. Similarly, extracted the prop_field_definitions setting for the SingleDirectoryComponent ComponentSource plugin out of its config schema type into a new config schema base type: https://git.drupalcode.org/project/experience_builder/-/merge_requests/5...
    4. Which then allowed me to quite simply/elegantly introduce the JsComponent ComponentSource: https://git.drupalcode.org/project/experience_builder/-/merge_requests/5...

    Next steps:

    1. auto-creating Component config entities based on JavaScriptComponent config entities. @longwave suggested automatically doing that whenever a JavaScriptComponent config entity is saved. I agree that makes sense. This would be a ::postSave() override in \Drupal\experience_builder\Entity\JavaScriptComponent.
    2. The easiest way to test is to expand \Drupal\Tests\experience_builder\Kernel\Config\ComponentTest β€” which was previously expanded in πŸ“Œ Add support for Blocks as Components Active to test Block-sourced Component config entities.
    3. πŸ“Œ Introduce unit test coverage for both ComponentSource plugins (Block + SDC) Active would make writing tests for this a hell of a lot simpler. ⚠️ But I think we explicitly should keep that out of scope to ensure we land this in the next few days rather than growing it. Let's add unit tests in #3475584.
    4. Docs: updating docs/components.md!
    5. I'm sure there's more, but those are the first steps :)

    P.S.: I did not test \Drupal\experience_builder\Plugin\ExperienceBuilder\ComponentSource\JsComponent::renderComponent() at all, I just matched what @larowlan did in \Drupal\Tests\experience_builder\Element\IslandTest. It likely needs updating πŸ˜…

  • πŸ‡§πŸ‡ͺBelgium wim leers Ghent πŸ‡§πŸ‡ͺπŸ‡ͺπŸ‡Ί

    Also merged in upstream and resolved conflicts, because this conflicted heavily with πŸ“Œ Remove references to 'props' outside of SDC - use 'inputs' instead Active .

    Should make for a smooth start for the next person 😊

  • πŸ‡§πŸ‡ͺBelgium wim leers Ghent πŸ‡§πŸ‡ͺπŸ‡ͺπŸ‡Ί

    wim leers β†’ changed the visibility of the branch 0.x to hidden.

  • πŸ‡§πŸ‡ͺBelgium wim leers Ghent πŸ‡§πŸ‡ͺπŸ‡ͺπŸ‡Ί

    SO great to see @larowlan continued where I left off 🀩

    But it looks like @larowlan tackled things I had not even identified in #22, so I can literally work on the list I created there πŸ˜„

  • πŸ‡§πŸ‡ͺBelgium wim leers Ghent πŸ‡§πŸ‡ͺπŸ‡ͺπŸ‡Ί

    It looks like I was wrong β†’ :

    • @larowlan did implement the "auto-create upon save" functionality, just not in ::postSave(), but in the new JavascriptComponentStorage πŸ‘ Only thing that needed changing: respect the status flag that ✨ Config entity for storing code components Active assigned a particular meaning to.
    • He also added explicit test coverage for it, in JavascriptComponentStorageTest. Still, updating testComponentCreation() the way that I did provides value: it ensures that identical props for an SDC and a "code component" result in the exact same prop_field_definitions πŸ‘

    I did find some problems though, and I've fixed those.

    I think I won’t be able to push this forward the next few hours. The only remaining thing I have partially (locally) is schema refinements, but there’s plenty more to do that won’t touch that.

  • πŸ‡§πŸ‡ͺBelgium wim leers Ghent πŸ‡§πŸ‡ͺπŸ‡ͺπŸ‡Ί

    Finished everything in #22, except for the docs (only a stub was added).

    Iterated on everything @larowlan did overnight, and between his work and mine, we surfaced a bunch of oversights in both ✨ Config entity for storing code components Active and the way that even the foundations of Component config entity type's support for ComponentSource-specific settings were not quite right yet. Which is understandable, because it's this issue that's introducing a 3rd ComponentSource plugin (and as the adage goes: an abstraction requires 3 concrete uses), and one that is quite different in some ways and almost the same in others!

    I've spent most of today increasing test coverage, and in turn revealing weaknesses in our validation and hence the reliability of the Component config entity type in combination with the new ComponentSource this MR is introducing. Those weaknesses are important to eradicate to remain on track for requirement 14. Configuration management.

    The one bit where I'm going quite strongly against what @larowlan did last night is https://git.drupalcode.org/project/experience_builder/-/merge_requests/5..., because by using ContribStrictConfigSchemaTestTrait, it became clear that all of those edge cases @larowlan was testing … actually are made impossible by the validation for the JavaScriptComponent config entity, and for good reason! (See the commit message.)

    I believe that now, especially after the source-specific test coverage (which revealed missing validation) and its fixes (A β€” note how this causes the exact same fails + B) everything is in place πŸ€“

    The only thing I have not reviewed at all is all rendering aspects: the astro island and asset generation. But for those, I'm happy to defer to @longwave & @larowlan. I want to get the config bits to be as robust as possible as soon as possible because they affect the data model/update path/overall stability β€” tweaks to the rendering can easily happen in subsequent MRs.

    So: approved this MR! Hopefully @larowlan & @longwave think this is RTBC by tomorrow 🀞😊

  • πŸ‡¦πŸ‡ΊAustralia larowlan πŸ‡¦πŸ‡ΊπŸ.au GMT+10

    FWIW

    @larowlan did implement the "auto-create upon save" functionality, just not in ::postSave(), but in the new JavascriptComponentStorage

    I did it in the storage handler because we have DI in storage handlers, in Entity::postSave we have to use \Drupal. We use this pattern fairly regularly for client projects and I think it is cleaner.

  • πŸ‡§πŸ‡ͺBelgium wim leers Ghent πŸ‡§πŸ‡ͺπŸ‡ͺπŸ‡Ί

    #28:: ah, of course! I somehow always forget that’s an option πŸ˜‡πŸ™ˆ

    Green and lots of commits overnight 🀩

    Reviewing!

  • πŸ‡§πŸ‡ͺBelgium wim leers Ghent πŸ‡§πŸ‡ͺπŸ‡ͺπŸ‡Ί

    @larowlan opened πŸ“Œ Allow component source plugins that don't have a plugin_id setting Active and πŸ“Œ [PP-1] Remove more component manager overrides Active .

    I opened πŸ“Œ [PP-1] Refactor GeneratedFieldExplicitInputUxComponentSourceBase and FieldForComponentSuggester to need only SDC's ComponentMetadata, not SDC plugin instances Postponed .

    The goal for this issue, confirmed by @balintbrews in #8, is this:

    @balintbrews: Did you intend for this issue to deliver end-to-end working code components, in the sense that:

    • my code components appear in the left-hand in the XB UI
    • I can drag them onto the canvas and see a preview
    • I can populate their inputs using a field widget, and the preview updates
    • I can save it

    ?

    Ideally, we'd have an end-to-end test. But I think given that 99% is identical to SDCs, we could get away with not having that, and a future issue could do that β€” the scope of this one is already enormous.

    I think that this MR could land if:

    1. manual testing shows it works
    2. the path is paved for a future end-to-end test, by ensuring that a JS component is already present and available for placing in e2e tests
  • πŸ‡§πŸ‡ͺBelgium wim leers Ghent πŸ‡§πŸ‡ͺπŸ‡ͺπŸ‡Ί

    Thanks to @longwave for getting the missing bits for Astro assets in place πŸ‘ He also tells me he has essentially only nits. 😊

    Good news all around, because:

    1. Manual testing shows it works:

      (for this demo, I only commented out the if ($this->configInstaller->isSyncing()) early return in JavascriptComponentStorage β€” to force a corresponding Component config entity to be created πŸ‘)
    2. Pure back-end test
    3. End-to-end test that verifies a code component can appear in the primary panel's "Library" tab and renders a preview on hover: (failed upon adding πŸ‘)
    4. End-to-end test that verifies a code component can be added to the canvas (failed upon adding πŸ‘)
    5. Made the end-to-end tests pass again by actually creating the expected JS component: https://git.drupalcode.org/project/experience_builder/-/pipelines/409263

    Given how many front-end issues this blocks (virtually everything in 🌱 [Meta] Plan for code components Active ), plus it following the exact same pattern as the pre-existing test logic, plus the vastness of this MR (~2500 LoC added, net 😳), I'm not awaiting FE approval for the end-to-end tests. It's too important to land this sooner rather than later!

    So: RTBC'ing. Only thing outstanding: docs. First: belated lunch.

  • πŸ‡³πŸ‡±Netherlands balintbrews Amsterdam, NL

    Crediting @effulgentsia, who had the vision for this plugin and using Astro islands.

  • Pipeline finished with Skipped
    4 months ago
    #409414
  • πŸ‡§πŸ‡ͺBelgium wim leers Ghent πŸ‡§πŸ‡ͺπŸ‡ͺπŸ‡Ί
  • Merge request !592Fix eslint. β†’ (Merged) created by longwave
  • πŸ‡ΊπŸ‡ΈUnited States effulgentsia
  • Automatically closed - issue fixed for 2 weeks with no activity.

Production build 0.71.5 2024