Make WordPress Core

Opened 2 years ago

Closed 6 months ago

Last modified 2 months ago

#56313 closed enhancement (fixed)

Add the ability to handle ES Modules and Import Maps

Reported by: idad5's profile idad5 Owned by: luisherranz's profile luisherranz
Milestone: 6.5 Priority: normal
Severity: normal Version:
Component: Script Loader Keywords: has-patch has-unit-tests needs-dev-note
Focuses: javascript, performance Cc:

Description

wp_enqueue_script should be augmented to add scripts with type="module" to pages.
Or a new function like wp_enqueue_module etc. should be created.

Also having the ability to ad a nomodule scrip-tag should be considered, even though ES6 ans ES2020 module support is very wide spread.

Especially ES2020 dynamic modules are a feature that really should be available for theme developers, as it is the best (standard conform) way to handle dynamically loading JS at runtime.

Change History (85)

#1 @costdev
2 years ago

  • Focuses javascript added
  • Keywords 2nd-opinion added
  • Severity changed from major to normal

Hi @idad5, welcome back to Trac!

I think it makes sense that this is in Core territory rather than a third-party plugin. This should be reasonably straightforward. - it can even just be done through filters:

<?php
// In Core, functions.php or a plugin:
add_filter( 'script_loader_tag', 'slug_add_module_support', 999, 2 );
function slug_add_module_support( $tag, $handle ) {
        if ( str_contains( $handle, 'nomodule' ) ) {
                return str_replace( '<script', '<script nomodule', $tag );
        }

        if ( str_contains( $handle, 'module' ) ) {
                if ( current_theme_supports( 'html5', 'script' ) ) {
                        return str_replace( '<script', '<script type="module"', $tag );
                } else {
                        return str_replace( 'text/javascript', 'module', $tag );
                }
        }

        return $tag;
}

// Then when a developer needs to enqueue module and nomodule scripts:
add_filter( 'wp_enqueue_scripts', 'slug_enqueue_scripts' );
function slug_enqueue_scripts() {
        $modules = array(
                'module-say-hello' => '/js/say-hello.js',
                'module-app'       => '/js/app.js',
                'nomodule-app'     => '/js/app.nomodule.js',
        );

        foreach ( $modules as $handle => $src ) {
                wp_enqueue_script( "slug-$handle", get_template_directory_uri() . $src );
        }
}
// say-hello.js
export const sayHello = () => console.log('hello')

// app.js
import { sayHello } from './say-hello.js'
sayHello()

// app.nomodule.js
function sayHello() {
        console.log('hello')
}
sayHello()

For now, I'll add the 2nd-opinion keyword to get some thoughts on the enhancement from other contributors.

#2 @neffff
17 months ago

I came here looking for this! I wanted to add an additional concern/wrinkle. In supporting modules (which as noted can be done with a simple filter). It seems adding support for the `importmap` script type would make this a great addition to the script loader.

This would allow modules to have configurable imports -- enabling sharing modules between front/back ends without having to transpile. I'm sure there are other benefits, but this was the one that sent me looking for that support.

#3 @joemcgill
13 months ago

  • Focuses performance added
  • Keywords needs-patch added; 2nd-opinion removed
  • Milestone changed from Awaiting Review to Future Release

I think we should consider implementing this. Browser support for this is already in a good place. Having an official way to register modules will allow us to dynamically keep unused javascript from being sent to clients.

Related:

#4 follow-up: @joostdevalk
13 months ago

+1 for implementing this in core, find myself working around it.

#5 @thomasprice61
11 months ago

+1
Any chance this can be added to the 6.4 scope?

#6 in reply to: ↑ 4 @Latz
10 months ago

+1

Took me almost one hour to find the needed filter ("script_loader_tag") and the code doesn't look too professional neither.

#7 @luisherranz
8 months ago

Just a heads up that we have started doing experiments to support modules and import maps:

https://github.com/WordPress/gutenberg/issues/55942

Feel free to review the experiments and share your opinion 🙂

#8 @gziolo
8 months ago

  • Owner set to luisherranz
  • Status changed from new to assigned

I'm assigning the ticket to @luisherranz, as he is actively working on it. Thank you so much for the initiative.

#9 @luisherranz
8 months ago

  • Summary changed from wp_enqueue_script needs to be able to handle modules (ES6/ES2020) to Add the ability to handle ES Modules and Import Maps

#10 @luisherranz
8 months ago

I've renamed the ticket title to make it more generic as it's not clear yet if this will be something added on top of wp_register_script or a new API like wp_register_module.

This ticket was mentioned in PR #5818 on WordPress/wordpress-develop by @luisherranz.


7 months ago
#11

  • Keywords has-patch has-unit-tests added; needs-patch removed

Trac ticket: https://core.trac.wordpress.org/ticket/56313

---

This PR proposes a new Modules API for WordPress, designed to work with native ES Modules and Import Maps. It introduces functions such as wp_register_module, and wp_enqueue_module.

The API aims to provide a familiar experience to the existing WP_Scripts class, offering similar functionality. However, it's not intended to duplicate the exact functionality of WP_Scripts; rather, it is carefully tailored to address the specific needs and capabilities of ES modules.

For this initial version, the current proposal is intentionally simplistic, covering only the essential features needed to work with ES modules. Other enhancements and optimizations can be added later as the community identifies additional requirements and use cases.

### Differences Between WP_Modules and WP_Scripts

  1. Dependency Specification:

With WP_Modules, the array of dependencies supports not just strings but also arrays that include the dependency type (static or dynamic). This design choice allows for future extensions of dependency properties, such as adding a version property to support "scopes" within import maps.

  1. Version Handling:

Versioning in WP_Modules accounts for SCRIPT_DEBUG by default. When in development mode (with SCRIPT_DEBUG set to true), versions are represented using timestamps, ensuring developers always load the most recent module version during development.

  1. Module Identifier:

Instead of a handle, WP_Modules utilizes the module identifier, aligning with the module identifiers used in JavaScript files and import maps.

  1. Enqueue and Register Functions:

Unlike wp_enqueue_script, the wp_enqueue_module function does not support registering the script simultaneously. This decision maintains a clear separation of concerns, where registration and enqueuing are distinct actions.

  1. Deregistration:

There is no equivalent of wp_deregister_module at this stage.

## API

### wp_register_module( $module_identifier, $src, $deps, $version )

Registers a module.

// Registers a module with dependencies and versioning.
wp_register_module(
  'my-module',
  '/path/to/my-module.js',
  array( 'static-dependency-1', 'static-dependency-2' ),
  '1.2.3'
);
// my-module.js
import { ... } from 'static-dependency-1';
import { ... } from 'static-dependency-2';

// ...
// Registers a module with a dynamic dependency.
wp_register_module(
  'my-module',
  '/path/to/my-module.js',
  array(
    'static-dependency',
    array(
      'id'   => 'dynamic-dependency',
      'type' => 'dynamic'
    ),
  )
);
// my-module.js
import { ... } from 'static-dependency';

// ...
const dynamicModule = await import('dynamic-dependency');

### wp_enqueue_module( $module_identifier )

Enqueues a module.

wp_enqueue_module( 'my-module' );

### wp_dequeue_module( $module_identifier )

Dequeues a module.

wp_dequeue_module( 'my-module' );

## Output

  • When modules are enqueued, they are printed within script tags containing type="module" attributes.
  • Additionally, static dependencies of enqueued modules utilize link tags with rel="modulepreload" attributes.
  • Lastly, an import map is generated and inserted using a <script type="importmap"> tag.

## Import Map Polyfill Requirement

Even though all the major browsers already support import maps, an import map polyfill is required until the percentage of users using old browser versions without import map support drops significantly.

This work is ongoing and will be added to this PR once it's ready. Contributors can follow the progress in the associated GitHub issue: guybedford/es-module-shims#406.

## Feedback

This Modules API proposal is open for feedback and further discussion. Please contribute ideas, potential use cases, or any other thoughts that could make this feature robust and broadly useful for the WordPress community. Please use the Trac Ticket or this pull request.

#12 @luisherranz
7 months ago

I've opened a PR with a proposal for an initial API. The description and API explanation were copied in the previous comment from GitHub, so I won't repeat them here.

https://github.com/WordPress/wordpress-develop/pull/5818

Please, contribute ideas, potential use cases, or any other thoughts either here or in the PR. Thank you!

This ticket was mentioned in Slack in #core-js by luisherranz. View the logs.


7 months ago

@luisherranz commented on PR #5818:


7 months ago
#14

I see that PHP 8.3 has some deprecations when using ReflectionProperty. I'll review and fix those.

#15 @cbravobernal
7 months ago

  • Milestone changed from Future Release to 6.5

#16 @jorbin
7 months ago

Thanks for the initial patch @luisherranz, a few questions and thoughts that come to me right away:

Can you explain why this reinvents dependency management rather than working off of WP_Dependencies or _WP_Dependency?

You mention not having registration and enqueuing together but don't include any rationale behind why extenders of WordPress should need to maintain two different mental models for how external files are loaded into the HTML.

I like the separation of dynamic and static modules, something that I think will be extremely helpful is if there is documentation to help guide developers in making the decision of which one to use.

A useful test would be for circular dependency.

#17 follow-up: @luisherranz
7 months ago

Can you explain why this reinvents dependency management rather than working off of WP_Dependencies or _WP_Dependency?

Oh, sure. Overall, I didn't see any benefit in doing so. Reusing WP_Dependencies resulted in a more complex code because there was practically no overlap as the dependency management we need for modules doesn't require the things that WP_Dependencies provides (like dependency printing ordering) but requires a few new things, like filtering by type, or, in the future, maintaining more than one dependency per module identifier to support scopes.

You mention not having registration and enqueuing together but don't include any rationale behind why extenders of WordPress should need to maintain two different mental models for how external files are loaded into the HTML.

This was suggested to me as an improvement over the Scripts API in the past. I don't remember who was at this moment, but I'll ask to see if that person can provide additional rationale.

To me, it made sense to separate both registration and enqueuing as a way to teach people that those are two separate steps, especially when working with modules where registered modules can't only end up being enqueued, but also preloaded or added to the import map.

I like the separation of dynamic and static modules, something that I think will be extremely helpful is if there is documentation to help guide developers in making the decision of which one to use.

Absolutely 🙂

A useful test would be for circular dependency.

I'll add one. Thank you!

@luisherranz commented on PR #5818:


7 months ago
#18

Thanks for the review, @swissspidy. I've made all the necessary changes 🙂

@luisherranz commented on PR #5818:


7 months ago
#19

When I was studying WP Scripts, I saw that wp_register_script returns false when the script is already registered and true otherwise:

if ( isset( $this->registered[ $handle ] ) ) {
  return false;
}
$this->registered[ $handle ] = new _WP_Dependency( $handle, $src, $deps, $ver, $args );
// ...
return true;

_code_

So, it works like this:

  • Returns true: the script has been successfully registered.
  • Returns false: the script hasn't been registered again, but it was successfully registered before, so _it is_ successfully registered.

In both cases, the script is successfully registered.

so we end up optimistically continuing as if the module is correctly registered

That's my point. It doesn't matter what WP Scripts returns, the script is always registered so we should be able to continue as if the script is correctly registered.

I'm not opposed to mimicking the true/false return of WP Scripts, but I'd like to understand the use cases first in case there's an opportunity for improvement in the API 🙂

So, is there a use case where that true/false return is actually useful information?

@jonsurrell commented on PR #5818:


7 months ago
#20

Good point. I agree, it doesn't seem very helpful to distinguish between "registered on this call" and "already registered" if the result is the same. If there's no real failure scenario, it seems fine to always return null.

@luisherranz commented on PR #5818:


7 months ago
#21

Ok. Thanks, Jon. Let's keep it as it is for now then, just to keep open the possibility of returning something useful in the future or until we discover of use case for the WP Scripts true/false behavior 🙂

@gziolo commented on PR #5818:


6 months ago
#22

Good point. I agree, it doesn't seem very helpful to distinguish between "registered on this call" and "already registered" if the result is the same. If there's no real failure scenario, it seems fine to always return null.

In the case of WP_Scripts, it bails out early when there is the same handle registered, and therefore prevents overriding the same handle by plugins. That's why `wp_deregister_script` exists, so it was still possible. Example usage in the Gutenberg plugin:

https://github.com/WordPress/gutenberg/blob/d3071ca03bf3db4cd4267b4d9eb48c5658491c0f/lib/blocks.php#L219-L232

It's going to be challenging to replicate the same code with the currently proposed approach when the only possibility is to dequeue the module. In the shared example, all these scripts might not be enqueued as it happens later in the process. However, maybe it's fine, as when using the proposed implementation for modules, the plugin can register the same handles again, and they will get successfuly overridden. The only difference would be that some unused modules would remain registered and hopefully never enqueued.

WP_Scripts also offers `wp_script_is` that allows checking whether the script was already registered or enqueued. In that sense, it's similar to what the false return value does when calling wp_register_script that tries to register a previously registered script handle.

@luisherranz commented on PR #5818:


6 months ago
#23

@felixarntz: thanks! It makes sense. I'll make those changes 🙂

@gziolo: So the use case for wp_deregister_script is to prevent the script from being enqueued in the situation where someone else is still calling wp_enqueue_script?

@gziolo commented on PR #5818:


6 months ago
#24

So the use case for wp_deregister_script is to prevent the script from being enqueued in the situation where someone else is still calling wp_enqueue_script?

Yes, this is how it works. There is a check that ensures that only registered scripts get printed to make it possible to safely deregister a script.

@flixos90 commented on PR #5818:


6 months ago
#25

WP_Scripts also offers `wp_script_is` that allows checking whether the script was already registered or enqueued. In that sense, it's similar to what the false return value does when calling wp_register_script that tries to register a previously registered script handle.

I agree providing an equivalent method (and wrapper function) to check whether a module has been registered or enqueued would be helpful. Either a wp_module_is( $module, $status ) or two individual wp_module_is_registered( $module ) and wp_module_is_enqueued( $module ).

Since this class implementation is technically separate from WP_Scripts and WP_Styles, I don't think we necessarily have to use the same approach for that. I don't think we explicitly support e.g. queue or to_do here anyway. FWIW I think the wp_script_is() function isn't great from an API perspective with requiring a "status" for something that should rather be distinct functions, so I'd prefer to go with two separate functions, one for registration check and one for enqueue check. Curious what you think.

In any case, if there are any concerns or pushback on this, it could also be added later; not a blocker from my perspective for the initial commit.

@gziolo commented on PR #5818:


6 months ago
#26

In any case, if there are any concerns or pushback on this, it could also be added later; not a blocker from my perspective for the initial commit.

Agreed. Not a blocker for the initial version. Let’s see if wp_module_is_registered() and friends turn out to be necessary when working on refinements in the Gutenberg plugin to use the version we discuss here. The same applies to wp_deregister_module. We can always add the polyfill in the Gutenberg plugin if there is no other way to correctly override view modules that WordPress core will ship in the future.

@swissspidy commented on PR #5818:


6 months ago
#27

  • Additionally, static dependencies of enqueued modules utilize link tags with rel="modulepreload" attributes.

From the MDN docs:

Preloading allows modules and their dependencies to be downloaded early, and can also significantly reduce the overall download and processing time. This is because it allows pages to fetch modules in parallel, instead of sequentially as each module is processed and its dependencies are discovered. _Note however that you can't just preload everything! What you choose to preload must be balanced against other operations that might then be starved, adversely affecting user experience._

Has any testing been done in this regard?

@luisherranz commented on PR #5818:


6 months ago
#28

Has any testing been done in this regard?

@swissspidy, the static dependencies of the enqueued modules are always required to load those enqueued modules, so the current logic preloads those static dependencies, but not the dynamic ones.

// foo.js
import bar from 'bar'; // static dependency, always used, required to load foo.js

const baz = await import( 'baz' ); // dynamic dependency, it may or may not be used

This also works recursively, so if bar has a static dependency, that dependency is also preloaded. This way, we ensure that everything that is required on page load is downloaded in parallel, without causing any waterfalls.

@swissspidy commented on PR #5818:


6 months ago
#29

Greg just brought up the topic of internationalization / localization. Has this been considered for this API here? I imagine people will want to use translations in modules. We'll probably need something similar to wp_set_script_translations, but also a solution that works well for dynamic dependencies.

@swissspidy commented on PR #5818:


6 months ago
#30

@felixarntz I think we'll want to have a story for localization first , see https://github.com/WordPress/wordpress-develop/pull/5818#issuecomment-1876870705

@flixos90 commented on PR #5818:


6 months ago
#31

That should definitely be a priority, but IMO not a blocker for a first commit.

@luisherranz commented on PR #5818:


6 months ago
#32

Thanks for the thorough review, Weston 🙂

I've addressed all the feedback except the one about renaming the class to WP_Script_Modules, which I like to discuss a bit more before doing so.

It's very likely that, in the future, once the import attributes are final, we'll add support for importing CSS and JSON:

import sheet from "some-styles-package" with { type: "css" };
document.adoptedStyleSheets = [sheet];

Preloads are a bit different and they can't be enqueued on their own (so that means we would need to allow people to specify the type), but other than that, they are pretty much like any other module.

It seems like JSON modules are officially referred as JSON modules (link), whereas CSS modules are officially referred as CSS module scripts ([) link] because there was a discussion to rename it from **CSS modules** to avoid confusion with the existing CSS Modules project.

So, I'm not saying that WP_Script_Modules is worse than WP_Modules, but that we should also think about the possibility of powering types of modules that are not scripts, and a class name that is suitable for all of them:

  • (Java)Script modules
  • CSS module scripts
  • JSON modules

What do you think?

#33 in reply to: ↑ 17 @jorbin
6 months ago

Replying to luisherranz:

You mention not having registration and enqueuing together but don't include any rationale behind why extenders of WordPress should need to maintain two different mental models for how external files are loaded into the HTML.

This was suggested to me as an improvement over the Scripts API in the past. I don't remember who was at this moment, but I'll ask to see if that person can provide additional rationale.

To me, it made sense to separate both registration and enqueuing as a way to teach people that those are two separate steps, especially when working with modules where registered modules can't only end up being enqueued, but also preloaded or added to the import map.

Forcing people to keep two different mental models in their head for how to interact with scripts on a page creates an unnecessary burden on the extenders of WordPress.

Why does it need to be two separate steps when it's not anywhere else in WordPress? Enqueuing a module should automatically register it just as enqueuing a script or a style does.

#34 @luisherranz
6 months ago

If we think long-term, the Scripts API is less relevant because it should be slowly taken over by the Modules API. But it's true that the Styles API is going to stick around and having two mental models will be confusing.

I'll make the necessary changes to the code.

@westonruter commented on PR #5818:


6 months ago
#35

@luisherranz Interesting. I wasn't aware of JSON and CSS also being on the roadmap for being modules. Nevertheless, I think WP_Script_Modules is still appropriate for them as well, as you listed they all mention "script":

  • JavaScript modules
  • CSS module scripts
  • JSON (JavaScript Object Notation) modules

Additionally, all of these modules alike end up getting printed in an importmap script, and more importantly, they all get used in a _script_ context. So they are all inherently "scripty".

@luisherranz commented on PR #5818:


6 months ago
#36

Ok! Thanks, Weston. I've renamed the class to WP_Script_Modules.

I've also added support to register the script with a single call to the enqueue function, as requested by @aaronjorbin in the ticket.

I still agree that it would have made sense to separate both registration and enqueuing as a way to teach people that those are two separate steps (especially when working with modules where registered modules can't only end up being enqueued, but also preloaded or added to the import map), but as @aaronjorbin pointed out, the wp_enqueue_style is still going to be relevant in the future, and it would be confusing to have different APIs for both:

wp_enqueue_style( 'my-style', '/styles.css' );
wp_register_module( 'my-module', '/module.js' );
wp_enqueue_module( 'my-module' );

So now, this also works:

wp_enqueue_style( 'my-style', '/styles.css' );
wp_enqueue_module( 'my-module', '/module.js' );

---

From my point of view, I think this is in a great state. I don't have plans to add any other changes to this initial version.

@Bernhard Reiter commented on PR #5818:


6 months ago
#37

Now that the WP_Modules class has been renamed to WP_Script_Modules -- should the wp_modules() function also be renamed to wp_script_modules() (for consistency)?

@luisherranz @westonruter

@luisherranz commented on PR #5818:


6 months ago
#38

I'd keep it as wp_modules to keep consistency with the register and enqueue functions.

@Bernhard Reiter commented on PR #5818:


6 months ago
#39

I'd keep it as wp_modules to keep consistency with the register and enqueue functions.

Works for me! And if we end up deciding otherwise, we can always change it in a follow-up 😊

#40 @Bernhard Reiter
6 months ago

  • Keywords commit added

#41 @Bernhard Reiter
6 months ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In 57269:

JavaScript: Add new Modules API.

This changeset adds a new API for WordPress, designed to work with native ES Modules and Import Maps. It introduces functions such as wp_register_module, and wp_enqueue_module.

The API aims to provide a familiar experience to the existing WP_Scripts class, offering similar functionality. However, it's not intended to duplicate the exact functionality of WP_Scripts; rather, it is carefully tailored to address the specific needs and capabilities of ES modules.

For this initial version, the current proposal is intentionally simplistic, covering only the essential features needed to work with ES modules. Other enhancements and optimizations can be added later as the community identifies additional requirements and use cases.

Differences Between WP_Script_Modules and WP_Scripts

Dependency Specification

With WP_Script_Modules, the array of dependencies supports not only strings but also arrays that include the dependency import type (static or dynamic). This design choice allows for future extensions of dependency properties, such as adding a version property to support "scopes" within import maps.

Module Identifier

Instead of a handle, WP_Script_Modules utilizes the module identifier, aligning with the module identifiers used in JavaScript files and import maps.

Deregistration

There is no equivalent of wp_deregister_script at this stage.

API

wp_register_module( $module_identifier, $src, $deps, $version )

Registers a module.

// Registers a module with dependencies and versioning.
wp_register_module(
  'my-module',
  '/path/to/my-module.js',
  array( 'static-dependency-1', 'static-dependency-2' ),
  '1.2.3'
);
// my-module.js
import { ... } from 'static-dependency-1';
import { ... } from 'static-dependency-2';

// ...
// Registers a module with a dynamic dependency.
wp_register_module(
  'my-module',
  '/path/to/my-module.js',
  array(
    'static-dependency',
    array(
      'id'     => 'dynamic-dependency',
      'import' => 'dynamic'
    ),
  )
);
// my-module.js
import { ... } from 'static-dependency';

// ...
const dynamicModule = await import('dynamic-dependency');

wp_enqueue_module( $module_identifier, $src, $deps, $version )

Enqueues a module. If a source is provided, it will also register the module.

wp_enqueue_module( 'my-module' );

wp_dequeue_module( $module_identifier )

Dequeues a module.

wp_dequeue_module( 'my-module' );

Output

  • When modules are enqueued, they are printed within script tags containing type="module" attributes.
  • Additionally, static dependencies of enqueued modules utilize link tags with rel="modulepreload" attributes.
  • Lastly, an import map is generated and inserted using a <script type="importmap"> tag.
<script type="module" src="/path/to/my-module.js" id="my-module"></script>
<link rel="modulepreload" href="/path/to/static-dependency.js" id="static-dependency" />
<script type="importmap">
  {
    "imports": {
      "static-dependency": "/path/to/static-dependency.js",
      "dynamic-dependency": "/path/to/dynamic-dependency.js"
    }
  }
</script>

Import Map Polyfill Requirement

Even though all major browsers already support import maps, an import map polyfill is required until the percentage of users using old browser versions without import map support drops significantly.

This work is ongoing and will be added once it's ready. Progress is tracked in #60232.

Props luisherranz, idad5, costdev, neffff, joemcgill, jorbin, swissspidy, jonsurrell, flixos90, gziolo, westonruter.
Fixes #56313.

@Bernhard Reiter commented on PR #5818:


6 months ago
#42

Committed to Core in https://core.trac.wordpress.org/changeset/57269.

Thank you, folks! This is big 🎉

@luisherranz commented on PR #5818:


6 months ago
#43

Follow-up tickets:

@flixos90 commented on PR #5818:


6 months ago
#44

It's great to see this committed! 🎉

Nevertheless, I think the naming topic was wrapped a bit quickly as it was only initiated 2 days ago by @westonruter, so that didn't leave much room for other people's thoughts before the commit. We can of course follow up if needed, but a final decision on naming needs to happen soon before more other things are implemented on top of the API.

I think we should continue the discussion, either here or in a new issue. I personally find it a confusing choice to go with WP_Script_Modules class but wp_*_module() function names. What's the rationale to do it differently between those two places? To me it doesn't seem to make sense.

  • Either we go with the more verbose script modules, in expectation ("future proof") that other kinds of modules will be made available via WordPress APIs in the future.
  • Or we stick to just "module" and agree on that, when we talk about "modules" in WordPress context, it's always JS modules.

#45 @Bernhard Reiter
6 months ago

In 57271:

Modules API: Fix indentation.

Follow-up [57269].

Props mukesh27.
See #56313.

#46 follow-up: @johnbillion
6 months ago

  • Keywords commit removed
  • Resolution fixed deleted
  • Status changed from closed to reopened

Nice work on this everyone! I'll reopen this ticket for some follow-up enhancements. Some of these may warrant separate tickets.

  1. The naming consideration raised in the discussion on https://github.com/WordPress/wordpress-develop/pull/5818
  2. This feature only works on the front end, not within the wp-admin area, is this intentional?
  3. Is it desirable or possible for a module to depend on a non-module script, or vice versa? Should it be?
  4. There appears to be no way to fetch the list of enqueued modules like it is for non-module scripts. I'd like to display the list of enqueued modules in my Query Monitor plugin but it doesn't seem possible. Perhaps the get_marked_for_enqueue() and get_dependencies() methods need to be public instead of private.

@westonruter commented on PR #5818:


6 months ago
#47

IMO, I think the functions should be wp_*_script_module() for consistency.

@gziolo commented on PR #5818:


6 months ago
#48

IMO, I think the functions should be wp_script_modules(), wp_register_script_module(), and wp_enqueue_script_module() for consistency.

It makes perfect sense after the class name changes were applied.

#49 @luisherranz
6 months ago

Is it desirable or possible for a module to depend on a non-module script, or vice versa? Should it be?

I think so, yes.

Copy-pasting here the opinion I shared about this topic in the Gutenberg Tracking Issue:

Migrating from scripts to modules is not possible for scripts that have external dependants due to backward compatibility issues. This means there are going to be scripts that will remain scripts forever.

To increase the adoption of modules, it makes sense that:

  • New modules can still use those remaining scripts as dependencies. Ideally, developers should be able to use modules instead of scripts, and the need for script dependencies should not be an obstacle to the decision to use modules instead of scripts.
  • Those remaining scripts can use new modules as dependencies. Ideally, all new packages should be modules from now on, and the need for existing scripts to use those modules as dependencies should not be an obstacle to the decision to use modules instead of scripts.

I also believe that the priority should be on the modules:

  • From now on, WordPress developers should be able to use modules instead of scripts for all their needs.
  • All the existing WordPress packages should be available to be consumed as modules from now on, even if some of them are linked to existing scripts under the hood.
  • Developers should not need to know which WordPress packages are linked to existing scripts. For them, everything should look like regular modules from now on.
  • Developers should not need to use a _script-related_ bundling tool while building their modules anymore.
  • Existing scripts should be able to import modules, but in this case, the API can be explicit: developers should know when they are importing a module instead of a script.

There appears to be no way to fetch the list of enqueued modules like it is for non-module scripts. I'd like to display the list of enqueued modules in my Query Monitor plugin but it doesn't seem possible. Perhaps the get_marked_for_enqueue() and get_dependencies() methods need to be public instead of private.

As stated in the PR description, this initial version was intentionally simplistic. That way, we start with something that provides the basic functionality, and build upon it incrementally. So if you want/need to add new capabilities, I think those should be discussed in new tickets.

This feature only works on the front end, not within the wp-admin area, is this intentional?

It's not. Feel free to add a new ticket to add hooks for the admin.

@azaozz commented on PR #5818:


6 months ago
#50

I think the functions should be wp_script_modules(), wp_register_script_module(), and wp_enqueue_script_module() for consistency.

Another +1 for this.

#51 in reply to: ↑ 46 @azaozz
6 months ago

Replying to johnbillion:

  1. The naming consideration raised in the discussion on https://github.com/WordPress/wordpress-develop/pull/5818

Yeah, this needs fixing. Thinking these https://github.com/WordPress/wordpress-develop/pull/5818#issuecomment-1887867378 are quite better.

  1. Is it desirable or possible for a module to depend on a non-module script, or vice versa? Should it be?

I'm "on the fence" on this but thinking perhaps not for now. A JS module depending on jQuery? Seems a bit like like mixing apples with oranges? :) Really want to see some "real life" examples of how that would work, and how useful it may be.

Also should this even be needed in PHP or can it be done better in the browser/with JS? Modules are loaded as either defer or async, so any hard-coded "old-style" JS (that is outputted before any modules in the HTML head) will always be available to them.

Last edited 6 months ago by azaozz (previous) (diff)

@luisherranz commented on PR #5818:


6 months ago
#52

Seems unanimous, then. I'll prepare a PR with the renaming.

This ticket was mentioned in PR #5869 on WordPress/wordpress-develop by @luisherranz.


6 months ago
#53

Trac ticket: https://core.trac.wordpress.org/ticket/56313

---

## What

Rename the public wp...module functions to wp...script_module:

  • wp_module() -> wp_script_module
  • wp_register_module() -> wp_register_script_module
  • wp_enqueue_module() -> wp_enqueue_script_module
  • wp_dequeue_module() -> wp_dequeue_script_module

## Why

Because it was decided after the patch was already committed:

@luisherranz commented on PR #5869:


6 months ago
#55

Sure 🙂

Do you want me to remove the module word from the print methods as well?

  • print_enqueued_modules -> print_enqueued
  • print_module_preloads -> print_preloads

Or maybe change them to script_module instead?

  • print_enqueued_modules -> print_enqueued_script_modules
  • print_module_preloads -> print_script_module_preloads

@flixos90 commented on PR #5869:


6 months ago
#56

@luisherranz I think the latter, let's use script_module(s) everywhere.

@luisherranz commented on PR #5869:


6 months ago
#57

I've renamed all the references to "modules" and replaced them with "script modules".

This ticket was mentioned in Slack in #core-performance by mukeshpanchal27. View the logs.


6 months ago

@luisherranz commented on PR #5869:


6 months ago
#59

Suggested edits to phpdoc and added PHP 7 typing

Thanks, @westonruter. I've applied all the necessary changes.

@luisherranz commented on PR #5869:


6 months ago
#60

Ok, everything looks good now. Thanks for your help, @westonruter 🙂

The PR is ready.

#61 @westonruter
6 months ago

FYI: I just filed #60320 to fix corrupted importmap JSON when HTML5 theme support is absent.

#62 @westonruter
6 months ago

  • Keywords commit added

PR is ready to commit.

#63 @dmsnell
6 months ago

In 57327:

Script Modules API: Rename wp_module to wp_script_module

Renames all mentions to "module" with "script module", including function names, comments, and tests.

Follow up to [57269]

The list of functions renamed are:

  • wp_module() -> wp_script_module().
  • wp_register_module() -> wp_register_script_module().
  • wp_enqueue_module() -> wp_enqueue_script_module().
  • wp_dequeue_module() -> wp_dequeue_script_module().
  • WP_Script_Modules::print_enqueued_modules() -> WP_Script_Modules::print_enqueued_script_modules().
  • WP_Script_Modules::print_module_preloads() -> WP_Script_Modules::print_script_module_preloads().

It also adds PHP 7 typing to all the functions and improves the types of the $deps argument of wp_register_script_module() and wp_enqueue_script_module() using @type.

Props luisherranz, idad5, costdev, nefff, joemcgill, jorbin, swisspidy, jonsurrel, flixos90, gziolo, westonruter, bernhard-reiter, kamranzafar4343
See #56313

@dmsnell commented on PR #5869:


6 months ago
#64

Merged in [57327]
3c4fbc353f5425a1ee2865d528f62a9225bc4a49

#65 @dmsnell
6 months ago

Leaving this open for other follow-ups.

#66 @luisherranz
6 months ago

Follow-up PR and Trac ticket to move the prints to the footer in classic themes:

#67 @westonruter
6 months ago

In 57341:

Script Loader: Only emit CDATA wrapper comments in wp_get_inline_script_tag() for JavaScript.

This avoids erroneously adding CDATA wrapper comments for non-JavaScript scripts, including those for JSON such as the importmap for script modules in #56313.

Props westonruter, flixos90, mukesh27, dmsnell.
See #56313.
Fixes #60320.

#68 @luisherranz
6 months ago

Small report:

#60240 has now been committed and modules should now be fully functional in both Block and Classic themes.

We still need to add the import map polyfill: #60232

This ticket was mentioned in PR #5945 on WordPress/wordpress-develop by @swissspidy.


6 months ago
#69

Only noticed this while looking at https://github.com/WordPress/wordpress-develop/pull/5888

wp_script_modules() is an odd function because a) it doesn't use a get_instance() method like elsewhere in core, b) it does more than just returning an instance. The first time you call it, it adds hooks.

Curious to hear thoughts on this alternative suggestion.

cc @luisherranz

Trac ticket: https://core.trac.wordpress.org/ticket/56313

@luisherranz commented on PR #5945:


6 months ago
#70

The idea of wp_script_modules() came from @felixarntz in these comments of the original PR:

I would recommend to make everything here non-static and instead introduce a simple wp_modules() function that encapsulates the default/main instance of the class, similar to how the existing wp_scripts() and wp_styles() functions do.

The idea to avoid loading/instantiating the class early in the WordPress bootstrap also came from Felix:

It would be great if you could introduce wrapper functions for these 3 as well (i.e. wp_print_import_map() etc.). This way you can reference the functions here, which avoids loading/instantiating the class early in the WordPress bootstrap process where it may not be needed.

Not a big deal, but an easy enough optimization IMO worth making.

Taking into account what Felix said, I thought it would be even nicer to add the hooks inside the first call to the wp_script_modules() function. That not only prevents the hooks from being added early in the WordPress bootstrap, but doesn't add them at all if nobody is using modules.

Again, as Felix said, it's probably not a big deal, just an optimization.

I don't have strong preferences over any of the options, though, so feel free to change it if you want 🙂

@swissspidy commented on PR #5945:


6 months ago
#71

Hmm I see.

Well in that case I suppose keeping the function is fine, but not a fine of adding the hooks in there as well.

sheebaawp commented on PR #5945:


6 months ago
#72

same issue facing again
wp_script_modules() is an odd function because a) it doesn't use a get_instance() method like elsewhere in core, b) it does more than just returning an instance. The first time you call it, it adds hooks.

This ticket was mentioned in Slack in #core-performance by mukeshpanchal27. View the logs.


6 months ago

#74 @swissspidy
6 months ago

In 57503:

Script Loader: Use a global variable in wp_script_modules().

This brings the function more in line with its related wp_scripts() and wp_styles() functions and makes it easier to reset the class instance in tests.

Props westonruter, luisherranz.
See #56313.

#76 @swissspidy
6 months ago

  • Keywords commit removed
  • Resolution set to fixed
  • Status changed from reopened to closed

@luisherranz commented on PR #5945:


6 months ago
#77

Could you please take a look at the class initialization at https://github.com/WordPress/wordpress-develop/pull/5953? I'm doing something similar there to what I did here, and I don't know what the correct way to do it would be.

Thanks 🙏

#78 @stevenlinx
5 months ago

  • Keywords needs-dev-note added

#79 follow-up: @gziolo
5 months ago

There is now a way to deregister a script module added with #60463:

<?php
function wp_deregister_script_module( string $id ) {
        wp_script_modules()->deregister( $id );
}

The question is, should we call it wp_deregister_script_module for consistency with scripts and styles. If we want to keep consistency, should we also rename all $id params with $handle and deregister() method with remove()?

#80 in reply to: ↑ 79 ; follow-up: @azaozz
5 months ago

Replying to gziolo:

The question is, should we call it wp_deregister_script_module for consistency with scripts and styles.

Imho this name is descriptive, sounds good, and seems consistent. Replacing deregister with remove would also work (imho both are quite good).

If we want to keep consistency, should we also rename all $id params with $handle

Yea $handle is used everywhere in (the old) script loader. However I've always had the feeling it is somewhat unclear what exactly it represents: something between "name" and "id"?

Agree that the current $id params may be a bit ambiguous. They were renamed in [57327] from $module_id. If they are renamed again, I'd suggest using the old name: $module_id as that seems better than $handle imho. Another possibility would be $script_module_id which is long but best describes what they are and how they are used.

This ticket was mentioned in PR #6160 on WordPress/wordpress-develop by @cbravobernal.


5 months ago
#81

Changes script module deregister internal function name to remove, to keep consistency with the other scripts and styles functions.

Trac ticket: https://core.trac.wordpress.org/ticket/56313

#82 in reply to: ↑ 80 @cbravobernal
5 months ago

Replying to azaozz:

Imho this name is descriptive, sounds good, and seems consistent. Replacing deregister with remove would also work (imho both are quite good).

I created a small PR to change that: https://github.com/WordPress/wordpress-develop/pull/6160

Agree that the current $id params may be a bit ambiguous. They were renamed in [57327] from $module_id. If they are renamed again, I'd suggest using the old name: $module_id as that seems better than $handle imho. Another possibility would be $script_module_id which is long but best describes what they are and how they are used.

If we consider $id more descriptive than $handler, imho, adding the script_module may not be necessary, as those functions are already inside a script-modules folder. (Although it is true that is not the best for search indexes).

But, while changing that, we could mess up with the dependencies array, which all of them are currently using 'id' key as reference.

<?php
// src/wp-includes/class-wp-script-modules.phpL73
$dependencies[] = array(
  'id'     => $dependency ['id'] ,
  'import' => isset( $dependency['import'] ) && 'dynamic' === $dependency['import'] ? 'dynamic' : 'static',
);

#83 @jonsurrell
5 months ago

I'm planning to handle the dev note for Script Modules.

#84 @jonsurrell
5 months ago

Regarding the $id parameter, $module_id or $script_module_id removes and doubt and is likely easier to find when searching. However, in the context of modules $id seems unlikely to be a significant source of confusion. My preference would be to maintain $id.

For precedent, scripts use $handle, not $script_handle.

<?php
function wp_register_script( $handle /*…*/ ) {}
function wp_register_script_module( string $id /*…*/ ) {}

Although it may be a poor comparison 🙂 I agree with azaozz that

[$handle] is somewhat unclear what exactly it represents: something between "name" and "id"?

@jonsurrell commented on PR #6160:


2 months ago
#85

This doesn't seem necessary at this point, especially now that it would change a public function that was released in 6.5.

Note: See TracTickets for help on using tickets.