Add Interactivity API runtime #49994
Conversation
|
Size Change: 0 B Total Size: 1.37 MB ℹ️ View Unchanged
|
|
Flaky tests detected in af55917. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/4885420802
|
gziolo
added
[Package] Block library
|
Curious on including this in the The block library can import from other packages, and I think having these in separate directories has some clear value too. |
|
Yes, but not yet. This preliminary version is only meant to be used internally by core blocks. We don't even want to expose it yet as experimental. |
luisherranz
removed request for
a team,
ockham,
spacedmonkey,
gziolo and
ajitbohra
There was a problem hiding this comment.
It's great to see the initial PR opening the Interactivity API in the Gutenberg plugin. I left feedback that concerns integration with the plugin and how it will impact the future development of interactive blocks.
I don't think there is anything blocking it, but we need to have a solid plan for the next steps that ensure that we have the integration of Interactive API with the plugin working seamlessly as we start adding interactive behaviors for individual blocks. My expectation would be that we work on the following task as a follow-up for this PR that makes it easier to use the runtime with PRs that build on top of it:
- moving the existing unit tests for the runtime and directives to the temporary directory
packages/block-library/src/util/interactivity - adding JSDoc comments that document the runtime code and directives
- ensuring that the front-end interactivity can be easily wired with core blocks by updating a single place in the codebase if possible
- every behavior added to the block is covered by a corresponding e2e test
| splitChunks: { | ||
| cacheGroups: { | ||
| vendors: { | ||
| name: 'vendors', |
There was a problem hiding this comment.
How should it be handled when a core block imports a new vendor script that isn't part of the runtime?
There was a problem hiding this comment.
That's a good question. 🤔 I guess we can modify the vendors' bundle's definition so it only includes the common libraries, i.e., preact, @preact/signals, etc., instead of all dependencies from node_modules.
| process.env.BABEL_CACHE_DIRECTORY || true, | ||
| babelrc: false, | ||
| configFile: false, | ||
| presets: [ |
There was a problem hiding this comment.
Does this Babel config respect the setting for browserslist that WordPress uses?
There was a problem hiding this comment.
Not sure, but we can change it. I'll take a look.
| module: { | ||
| rules: [ | ||
| { | ||
| test: /\.(j|t)sx?$/, |
There was a problem hiding this comment.
Does it work with TypeScript out of the box now? I think you need to include @babel/preset-typescript. See #28879 for reference.
There was a problem hiding this comment.
Let's leave TypeScript apart for now. I'll remove it from the test property.
There was a problem hiding this comment.
It would work as we don't use TypeScript for core blocks, as far as I know. We can add TS support later.
| registerDirectives(); | ||
| init(); | ||
| // eslint-disable-next-line no-console | ||
| console.log( 'Interactivity API started' ); |
There was a problem hiding this comment.
Should this printing be guarded to be present only in the dev mode?
There was a problem hiding this comment.
That would be nice. What is the appropriate way to check we are in dev mode in the browser?
There was a problem hiding this comment.
This is a related conversation: WordPress/block-interactivity-experiments#222
But in this case, this console.log is just a quick way to make sure the Interactivity API loads correctly now that we are integrating everything together. It won't add much value when things "just work", so we can remove it later.
| "escape-html": "^1.0.3", | ||
| "fast-average-color": "^9.1.1", | ||
| "fast-deep-equal": "^3.1.3", | ||
| "lodash": "^4.17.21", | ||
| "memize": "^1.1.0", | ||
| "micromodal": "^0.4.10", | ||
| "preact": "^10.13.2", |
There was a problem hiding this comment.
Noting that react is set as a peer dependency to make integrating with 3rd party projects easier. Would it make sense to follow the same approach for preact?
|
Great feedback, thanks Greg 🙂
I don't think it's worth optimizing that at this moment because it will change once we expose the runtime for experimental use and that shouldn't take long. |
Fair enough, it should be way easier to draw conclusions once we have 2-3 core blocks working with the runtime 👍🏻 In fact, there are more comments that I left that might be premature, but now we have it documented we can revisit it later when we make Interactivity API approach the default approach for core blocks. |
Fair, but I would hope we would first seek to split these before potentially merging. At least, it's been the case that for many things it's near impossible to extract after the fact, particularly because of the need to design the decoupling into the interfaces. This is related to the packaging and code organization question, and I know that there's a fairly overwhelming preference in the project to isolate functionality into separate domains. It can still be possible to lock down the code even if it lives in a separate directory. Just some food for thought. |
gziolo
force-pushed
the
add-interactivity-runtime
branch
3 times, most recently
from
757926b to
3d94473
Compare
This is still pretty basic, just to check that it works. Co-authored-by: David Arenas <david.arenas@automattic.com>
Co-authored-by: David Arenas <david.arenas@automattic.com>
Co-authored-by: David Arenas <david.arenas@automattic.com>
Co-authored-by: David Arenas <david.arenas@automattic.com>
Co-authored-by: David Arenas <david.arenas@automattic.com>
Co-authored-by: David Arenas <david.arenas@automattic.com>
Co-authored-by: David Arenas <david.arenas@automattic.com>
Co-authored-by: David Arenas <david.arenas@automattic.com>
Co-authored-by: Weston Ruter <westonruter@google.com>
…ripts This replicates the same logic in WordPress core: https://github.com/WordPress/wordpress-develop/blob/a5f3087f6b5d9c52dbe67ed247165dc32427c57d/src/wp-includes/script-loader.php#L306-L308
gziolo
force-pushed
the
add-interactivity-runtime
branch
from
06cdbd9 to
0cdab08
Compare
I fixed it with 0cdab08. I'm not sure why that code was enforcing including I replicated the logic from WordPress core that resolves the issue and ensures that translations still work where applicable: I also rebased the branch to include the fix that resolves the issue with the Navigation block not working correctly on the front end when using the version from The post editor when using Polish translations (I had to update translation using the admin interface to see them correctly applied):
|
There was a problem hiding this comment.
I think this PR is now in good shape to land in the trunk. I summarized the latest status in #49994 (comment) and followed-up with #49994 (comment).
There are still some comments to address left by @felixarntz and @dmsnell that it would be great to address before merging it. I applied a couple of commits to this branch, so I would appreciate some cross-checking from someone else. I'm mostly expressing that I don't see any blockers for this PR, but there is still follow-up work that can happen while we expose the Interactivity API behind the feature flag setting to the users.
|
Thanks for the review, @gziolo 😊 I've been testing the latest changes with @SantosGuillamot using the Navigation block on #50041, and everything seems to be working now. |
What?
Adds the Interactivity API runtime to the
block-librarypackage.Tracking issue: #49868
Why?
To provide a basic runtime in order to start experimenting with the Interactivity API in some of the core blocks.
How?
/block-library/src/utilsfolder.deferattribute in all Interactivity API scripts.interactivity/index.jsfile insideEffects, as it's the one that inits the Interactivity API runtime (I forgot to mention it in the video 😅).Here's a video explaining the changes done and how to start using the Interactivity API in core blocks.
Add Interactivity API runtime by DAreRodz · Pull Request #49994 · WordPress/gutenberg - 21 April 2023 - Watch Video