-
Notifications
You must be signed in to change notification settings - Fork 4.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Experiment: A simple Modules API with no server dependency graph #56092
Experiment: A simple Modules API with no server dependency graph #56092
Conversation
if ( $should_load_view_script ) { | ||
gutenberg_enqueue_module( | ||
'@wordpress/block-library/navigation-block', | ||
'/wp-content/plugins/gutenberg/build/interactivity/navigation.min.js' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you think we can avoid the file source from the enqueue function? I expect enqueued modules to be registered before hand no? (I know the current scripts API also allows this but it feels weird to me)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure. I'll do that.
One thing that feels a bit weird to me, though, is to add an entry to the import map for the enqueued modules because they rarely export anything. So I wonder if register
and enqueue
should be separate things, one adds entries to the import map and the other adds <script>
tags. Something like:
gutenberg_include_module
: adds the module to the import mapgutenberg_enqueue_module
: adds the module to the header using<script>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see what you mean, I'm not sure at the moment. I also wonder if there are scripts that could be entry points but also dependencies at the same time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I also wonder if there are scripts that could be entry points but also dependencies at the same time.
I think it's a possibility, but it's going to be uncommon.
* is set to true, it uses the timestamp instead. | ||
* } | ||
*/ | ||
public static function register( $module_identifier, $src, $args = array() ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the basic version (no server graph), I was wondering if we could support some kind of "flags" or something to say that a script is a backend script, frontend script or both. (It's a small optimization as there's a lot of backend script that are not meant to be loaded in the frontend)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure. I don't think it makes sense to have a version of the API that doesn't do that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added in 11b4e1b.
@@ -29,6 +29,7 @@ class Gutenberg_Modules { | |||
* | |||
* @param string $module_identifier The identifier of the module. Should be unique. It will be used in the final import map. | |||
* @param string $src Full URL of the module, or path of the script relative to the WordPress root directory. | |||
* @param string $usage Specifies where the module would be used. Can be 'admin', 'frontend', or 'both'. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the same script can be probably be used in both frontend and backend. I also wonder if we'd have more "usage" in the future. Do you think a string is enough or should it be an array? I'm not very opinionated, just asking the question I guess.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense. I changed it so now it accepts a string or an array of strings: e0201ce
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is cool. Thanks for the work here. I'm looking forward to seeing this IRL.
522268e
to
c5b2c83
Compare
also related to the thread above
WDYT, of making it more explicit. Instead of making up a new word for a high-level concept of us "registering"/"including", make the function state what we actually do.
(No module was referenced above) Personally, I find "include" more relevant to the behavior of PHP include statement or HTML includes - meaning it actually stamps/includes/pastes the content to be processed. What we do here - registering an entry in import - is just adding an entry to a map for a potential future reference, which may never be used. I don't mean we need to cover all the aliasing, remapping, and all features of import maps in the first iteration. But let's not limit ourselves and users without a good reason. For example, just by changing the name, and leaving the implementation more-or-less the same we can have unlock use-cases like:
gutenberg_register_import_map_entry( '@automatewoo/bundled/', '/wp-content/plugins/automatewoo/build/...');
// Somewhere in AutomateWoo codebase:
import { Button } from '@wordpress/components';
import { Button as PatchedButton } from '@automatewoo/bundled/@wordpress/components'; To cover the bundled-vs-extracted logic and developer experience currently covered by the Dependency Extraction Webpack Plugin. #35630
gutenberg_register_import_map_entry( '@automatewoo/static-data/', '/wp-content/plugins/automatewoo/data/');
// ...
import { version, minWP } from '@automatewoo/static-data/extension-info';
console.log( `You're running AutomateWoo ${ version }, it requires at least ${ minWP }.` ); Without a need to "register" every single file under |
Hey Tomek, that's indeed an interesting approach. Thanks for sharing 🙂 However, I see some issues for which I don't think it would be good to expose an API like that in WordPress. Remember that WordPress is a platform where code ownership is divided among different people (core, different plugins, theme…), which affects how we need to think about these APIs. Let me elaborate. Cache InvalidationWhen you update WordPress or a plugin, they might include newer versions of some of the JavaScript files. Other JavaScript files might rely on those updated versions to work correctly. Most hosting providers configure their servers to treat JavaScript files as immutables, and they send those cache headers to the browser. This means that in WordPress, the URLs of each JavaScript file need to be unique, or your site might not function correctly when a user who has visited previously the site, visits it again but doesn't get updated URLs. If we expose a low-level API and people can add entire folders to the import map, the filenames need to be hardcoded in the JavaScript files, where we cannot make them unique: // PHP
gutenberg_register_import_map_entry(
"@automatewoo/",
"/wp-content/plugins/automatewoo/build/"
);
// JS
import { something } from "@automatewoo/some-folder/some-file.js"; // filename hardcoded The only way to invalidate the cache would be to include a build hash or version in the build folder name. But forcing every plugin author to use that complex build configuration doesn't seem like a good idea, especially when people are already used to delegating that management to WordPress. Filename Backward CompatibilityBy hardcoding file paths in the JavaScript files, we would lock them into those specific paths/names. In the future, if someone decides to rename or relocate a file, any JS code that references the old path/name will break. That practice will lead to a fragile architecture where the risk of breaking changes is high, as the system lacks the flexibility needed for maintaining and refactoring the codebase. For example: // Original import
import { utilFunction } from "@automatewoo/some-folder/utilities.js";
// If we move utilities.js to another folder or rename it, this path is invalid. Dependency PreloadingPreloading is important for performance, as it helps avoid initial waterfalls by allowing the browser to start downloading static dependencies early on. Introducing a low-level API that could be used to register folders (as opposed to files) might complicate this process if dependencies (and dependencies of those dependencies) are unknown by the server. <link rel="modulepreload" href="/path/to/dependency.js" /> Exposing an additional API exclusively for preloads may work fine for direct static dependencies, but if those dependencies have other dependencies, preloading them becomes non-trivial. You'd have to know and manage the entire dependency tree manually, which can become cumbersome and error-prone, as any update in an external plugin could change their dependencies without notice. This is not a problem exclusive to the low-level API you suggested, as this first experiment is also not able to handle preloads, but it's another issue that will make it difficult to adopt it in more complex implementations like in the second or third experiments. So, while I agree that the idea of using a low-level API for managing import maps might seem appealing for its simplicity and directness, and would be an excellent choice in other platforms, I believe that, in WordPress, it might give the wrong incentives to people and introduce several significant maintenance and usability issues regarding cache management, backward compatibility, and dependency preloading. Let me know what you think 🙂 |
Thank you for the elaborate answer :) Forgive me that I'd start philosophically ;) My main idea was that as we're still in the early stage of experimentation, we could allow ourselves to start simple. Exposing APIs that may be abused and used in suboptimal ways if someone would like to go in a different direction than we imagine. Bringing it back to our ground: if we deliver (experimental) Re: Cache Invalidation
My point was not to make the API to allow folders in import maps but to make a low-level API that would interact with the import map more directly. Including allowing solving versions in a self-descriptive way. Adding import map entry can be used precisely to map a file name, to the versioned one: // PHP
gutenberg_register_import_map_entry(
"@automatewoo/some/file.js",
"/wp-content/plugins/automatewoo/build/some/file" . get_version()
);
// JS
import { something } from "@automatewoo/some/file.js";
I cannot agree more. Especially, that as a Woo extension developer, I feel the pain of a specific build process being forced on me. Re: Filename Backward Compatibility
My proposal did not introduce any more hardcoding paths in JS than the original.
One can argue, that hardcoding JS file paths in PHP is equally a bad practise, as it makes the code hard to maintain. Speaking of a "platform where code ownership is divided among different people … which affects how we need to think about these APIs" When Modules were introduced to the Web ;) They were also introduced without bare identifiers, separately from import maps, AFAIK to allow a faster implementation and more flexibility. Maybe we can follow that path. Re: Dependency PreloadingThat's the problem the API proposed by me will definitely not help to solve. But well, that's the point of being "low-level" it does not have to include all the features and solve all the cases. Whereas
Is definitely an important point. However, personally, I'm not a fan of handling the dependency graph ourselves. It's pretty complex and expensive work. It means we'd be re-implementing in PHP what JS engines already do. (I see a point and value, of preloading, and don't have anything better to propose. Maybe except of employing actual already made implementation). I also see the importance of "it might give the wrong incentives to people". So whatever we actually ship should support the right incentives and maintainability. But are when there already? Said all the above, I see a list of valid reasons not to follow my proposal. I just hope that our high-level-first approach will not prematurely limit WP Platform and community possibilities, innovation, and it will not actually impose too many constraints and inconveniences on the developers. If we are already trying to support high-level things like "issues regarding cache management, backward compatibility, and dependency preloading.". I'd strongly vote to add the dependency extraction and its exceptions & extensions to the list. In my understanding import maps, by design are meant to solve the problems DEWP was created to solve. It's more sustainable to expect developers in the WordPress ecosystem to understand the native Web feature of import maps. Instead of forcing them to use Webpack, with our custom, complex build process, and expect to learn our own tool. Plus, we would not have to document, and maintain it ;) |
This is now part of WordPress core 🎉 The final approach is documented in the dev note: Script Modules in 6.5. |
What?
This is the first of a series of experiments to understand the requirements of a hypothetical Modules API. More info in the Tracking Issue:
This experiment aims to implement the simplest possible API, with these characteristics:
<script>
tag in the head.Why?
To understand the requirements, we can start with the simplest possible API and test it out with the interactive blocks.
How?
By leveraging a new
Gutenberg_Modules
class that acts as a singleton and a couple of functions to expose the functionality:gutenberg_register_module
to register modules and add entries to the import map.gutenberg_enqueue_module
to enqueue modules.Then, we print two times using the
wp_head
action:Testing Instructions
type="module"
are loaded by the interactive blocks.@wordpress/interactivity
module is loaded by the other modules usingimport ... from "@wordpress/interactivity"
.