Make WordPress Core

Opened 12 months ago

Last modified 4 days ago

#60647 reopened feature request

Script Modules: Allow modules to depend on existing WordPress scripts

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

Description

Script modules cannot depend on existing scripts such as wp-url, wp-i18n, or wp-api-fetch.

Script modules should be able to leverage the functionality available in existing scripts.

Change History (61)

#1 @johnbillion
12 months ago

  • Version set to trunk

#2 @jonsurrell
12 months ago

I've spent a good amount of considering this and I'll share my ideas. First some concepts and constraints followed by ideas for implementation. I welcome questions, challenges, and feedback.


Backwards compatibility is very important. Therefore, all of the currently available scripts need to remain available for the foreseeable future.

Many existing scripts have side effects when they're loaded, such as initialization of a store. They may also have state or singletons that do not behave as expected when duplicated. See Gutenberg issue #8981 for details.

Scripts share functionality by attaching variables to the window object to expose them, they're accessible through the global namespace. For example. wp-api-fetch is accessible through window.wp.apiFetch.

Scripts can use modules through "dynamic import" (`import()`). import() is an async function (returns a Promise), implying that scripts using modules necessarily become async.

Modules can trivially access script functionality as long as the order of evaluation is correct (window.wp.apiFetch cannot be accessed before the wp-api-fetch script has been fetched and evaluated).


Given

  • Scripts need to remain.
  • Scripts should not be duplicated.
  • Modules can use scripts easily via globals.
  • Scripts using modules requires promises.

Some possible approaches:

First, modules could depend on scripts. Modules can access scripts through globals, so all we need is for dependencies to correctly enqueue scripts. This should be a simple change but sacrifices the developer experience. Folks need to know whether they're using a module or a script, and they need to include the correct dependency and use it in the correct way: module dependencies are imported, script dependencies are used through globals.

// Our dependent module's has a complicated dependencies array:
$dependencies = array(
  '@wordpress/interactivity',
  // Somehow we declare a script dependency (this form is not currently valid)
  array( 'import' => 'script', 'id' => 'wp-api-fetch' ),
);

// In the dependent module, we have a mix of imports and globals:
import * as interactivity from '@wordpress/interactivity';
const apiFetch = window.wp.apiFetch;

Another approach is to have proxy modules that export the globals provided by a script. The proxy modules would still need to correctly enqueue the associated scripts —there is a module->script dependency— but this would be handled for Core scripts and not a public, maintaining a clear separation between modules and scripts. The developer experience here is improved, instead of depending on some scripts and some modules, modules only depend on modules and developers would not need to access globals, they'd only use import.

Here's what some proxy modules might look like:

// Module wrapper for wp-api-fetch script
// The @wordpress/api-fetch package uses a default export
const apiFetch = window.wp.apiFetch;
export default apiFetch;

// Module wrapper for wp-url script
// The @wordpress/url package uses named exports
export const addQueryArgs = window.wp.url.addQueryArgs
export const getPath = window.wp.url.getPath
export const isURL = window.wp.url.isURL
// etc…

The advantage to using these proxy modules is improved developer experience:

// Our dependent module's dependencies are simpler:
$dependencies = array( '@wordpress/interactivity', '@wordpress/api-fetch' );

// In the dependent module, we only use imports:
import apiFetch from '@wordpress/api-fetch';
import { addQueryArgs } from '@wordpress/url';

The proxy module approach also opens up a potential enhancement. WordPress could include a proxy module and a full module version of scripts. When preparing the modules, we could check whether the script is enqueued or not. If the script is enqueued, we use the proxy module backed by the script. If the script is not enqueued, we use the full module version and the script does not need to be enqueued. This is a path where scripts can remain available but are largely replaced by modules without sacrificing backwards compatibility.

There are some drawbacks to the module proxy approach, mostly that we have an additional request for the proxy module. One solution could be to print the proxy module inline immediately after its associated module (or after the importmap if that's not been printed yet). The proxy modules are essentially lists of exports so are likely small in general.

Another potential downside is that this proposal focused on core scripts, it's not focused on extenders. If the approach works well, we can consider adding the appropriate extension points for extenders to follow the same approach.

#3 follow-up: @gziolo
12 months ago

Thank you for sharing initial ideas, @jonsurrell.

Folks need to know whether they're using a module or a script, and they need to include the correct dependency and use it in the correct way: module dependencies are imported, script dependencies are used through globals.

This approach would be manual as we don't have any automated way to detect wp globals and turn them into the list of dependencies even when using @wordpress/scripts tooling.

Another challenge, unrelated to the approach proposed, is ensuring that scripts and the import map are printed in the correct order so that the browser can resolve everything correctly. At the moment, ES Modules and regular scripts don't know anything about themselves.

#4 in reply to: ↑ 3 @jonsurrell
12 months ago

Replying to jonsurrell:

There are some drawbacks to the module proxy approach, mostly that we have an additional request for the proxy module. One solution could be to print the proxy module inline …

I don't believe it's possible to print a module with exports inline because there is no way to refer to it later, it doesn't have a name or URL that other modules can import.

#5 @jonsurrell
12 months ago

One concern with the proxy module approach was around the ordering of script execution. For example, the proposed proxy module @wordpress/a11y has an implicit dependency on the wp-a11y script. wp-a11y exposes required global variables and therefore must be executed before @wordpress/a11y.

I've done some testing and research and it appears that the ordering of scripts printed on the page will not be a problem. In summary, if the scripts are not async or defer (by default they are not), and modules are not async (by default they are not), they will executed in the order we require.


Enqueued scripts are printed as script tags without async or defer attributes. This means they "are fetched and executed immediately before the browser continues to parse the page." Enqueued scripts will be fetched and executed in the order they appear on the page.

Modules (<script type="module">) are always treated as `defer`, which means:

the script is meant to be executed after the document has been parsed.

If we put those together, we can see that scripts are executed before parsing continues, while modules are executed after the document has been parsed. This means that modules can depend on scripts independent of the ordering of script tags on the page, because scripts execute before modules.

Last edited 12 months ago by jonsurrell (previous) (diff)

#6 @gziolo
12 months ago

  • Milestone changed from Awaiting Review to 6.6

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


12 months ago

This ticket was mentioned in Slack in #core-editor by gziolo. View the logs.


11 months ago

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


11 months ago
#9

  • Keywords has-patch added

#10 follow-up: @jonsurrell
11 months ago

I've spent more time thinking about this. I see two options:

The first option is to allow modules to depend on scripts. This seems to be a fairly simple and safe change. Scripts continue to exist as scripts for the foreseeable future. Modules would use the scripts as e.g. window.wp.apiFetch. We can adapt existing tooling to help with this so folks can continue to author code like import apiFetch from '@wordpress/api-fetch' and compile it with the expected dependency and result (wp-api-fetch script dependency and const apiFetch = window.wp.apiFetch).

The downside with this first option is that it does not provide a transition strategy from scripts to modules and we sacrifice some of the potential benefits of modules like deferred or conditional/on-demand loading of modules.


The alternative is to expose scripts as modules somehow. Most of the modern scripts available in WordPress are authored as modules, then compiled via webpack as "libraries" — they expose their exports as global values, e.g. window.wp.apiFetch.

The proxy modules I've described in earlier comments provide a mechanism where a script dependency could be used when present or a module used on-demand when necessary. However, the proxy modules are worse in most ways: they're mostly overhead that only serve to use a module or script. The benefit would come if we can determine that a given script is not used on a page so that its corresponding module can be used directly without a proxy. If we cannot meet those conditions or only meet them in extremely rare conditions, then I don't see reason to pursue this approach.

I'll describe some scenarios to explain my thinking:

  • wp-api-fetch script is present. @wordpress/api-fetch module is not a dependency. Behavior is unchanged.
  • wp-api-fetch script is present. @wordpress/api-fetch module is a dependency. In this case @wordpress/api-fetch should point to the proxy module so that the wp-api-fetch script is used and not duplicated as a module.
  • wp-api-fetch script is not present. @wordpress/api-fetch module is a dependency. In this case @wordpress/api-fetch should point to the full @wordpress/api-fetch module to take advantage of the benefits of ES modules.

The difficulty of this comes from knowing whether a given script will be added to a page when we print the module importmap, that's the moment when we must provide a URL pointing to a proxy module or a real module. The importmap must be present on the page before any script type=module or link rel=modulepreload tags. Ideally the module preloads and the importmap are printed early in the page, so if scripts are enqueued late this information may not be available. This was a problem with the Modules API and classic themes.

Maybe proxy modules could always be used with classic themes, but block-based themes could use this strategy of using the proxy or a real module depending on whether a script is enqueued.

#11 @jonsurrell
11 months ago

There's an additional difficulty with some scripts when trying to use them as modules. Some scripts depend on special initialization as inline scripts. There's no obvious way to do this with modules without sacrificing some of the benefits of modules.

For example, wp-api-fetch script is initialized with an inline script like this:

wp.apiFetch.nonceMiddleware = wp.apiFetch.createNonceMiddleware( nonce );
// …more setup code

Where nonce is a server generated nonce for the REST API. This can be achieved in the same way with modules by using script type=module tag and importing @wordpress/api-fetch, but the module will be necessarily required which is undesirable. Ideally the module is only downloaded and initialized if and when it's needed.

A better solution may be to modify wp-api-fetch so that rather than the server injecting imperative code to set it up, instead it searches for some global variables and performs its own setup/initialization when necessary.

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


11 months ago

#13 in reply to: ↑ 10 ; follow-up: @azaozz
11 months ago

Replying to jonsurrell:

The first option is to allow modules to depend on scripts. This seems to be a fairly simple and safe change.

Big +1. Thinking any "mixing" of modules and "ordinary, old-style" scripts should be kept to a minimum, and the simpler it is handled -- the better.

The downside with this first option is that it does not provide a transition strategy from scripts to modules and we sacrifice some of the potential benefits of modules like deferred or conditional/on-demand loading of modules.

Frankly I'm not sure if the WP old-style scripts can be transitioned to modules in many (most?) cases. The reason it that this would break any (old style) scripts added by plugins that depend on them. So thinking this is a no-issue for now, and perhaps won't be for a long time, if ever.

Seems the way forward would be to deprecate the old-style scripts and replace them with modules, then load them if/when a plugin enqueues them. (Of course, for this the module would have to be able to co-exist with the old-style script, and work together with it. This would probably be quite tricky. But lets look when we get there.)

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

#14 in reply to: ↑ 13 @cbravobernal
11 months ago

Replying to azaozz:

Seems the way forward would be to deprecate the old-style scripts and replace them with modules, then load them if/when a plugin enqueues them. (Of course, for this the module would have to be able to co-exist with the old-style script, and work together with it. This would probably be quite tricky. But lets look when we get there.)

Can we apply a POC of this solution first to the most used ones?

Applying just to i18n, api-fetch or core-data. As far as I know, community is asking to use core-data in the frontend too, and can be also a reason to "update it" to be frontend compatible and module compatible.

#15 follow-ups: @youknowriad
11 months ago

We've discussed this issue a bit with @jonsurrell while no solutions emerged, I wanted to add some opinion here.

First, I'm actually not convinced at all that we should allow modules to depend on scripts (or to use scripts for that matter). I'm not convinced that the use-cases are too big for existing scripts. I do see folks wanting to use apiFetch or internationalization (probably a different one than wp-i18n), date maybe but these are fairly small scripts.

I think it's probably a mistake to try to use scripts in modules for different reasons:

  • A lot of these scripts are initialized server-side using inline scripts that are contextual. If I load wp-api-fetch in the editor or the dashboard, I won't get the same inline scripts. For modules, the opposite is true, the modules is always the same. This property of modules is a good thing in terms of architecture (separation between server and client) and making modules depend on scripts would be hard (which inline scripts to use) and create the issue of making the modules context dependent.
  • A lot of scripts have sub dependencies: polyfills, not certain all of this history is needed for the modules.
  • Add scripts as dependencies of modules, meaning that these scripts will always be printed in the page even if the module is not needed (will be async loaded later), which means the main selling point of modules becomes mute. We'll be shooting ourselves in the foot.

For these reasons, I think it's not worth it to try to load scripts in modules. I think we'd better try having build scripts that creates a script and a module for some selected packages. And I can even see some differences between the script and the module version of something. For instance configuring the translations shouldn't be done in the same way to wp-i18n, we'd have to load contextual translations instead (lazy load them as needed rather than print them server side...)

While this doesn't bring a solution, I think my best advice here would be to not try to solve this in a generic way, instead we should focus on concrete use-cases. I see more value personally (for a start) to be able to use modules within scripts (lazy load modules). For example a very concrete use-case I always bring is to be able to load a "codemirror" module in the editor for the code view for instance. I'd like editPost to be able to do import( '@wordpress/codemirror' ) when needed.

There might be other use-cases we can try with as well, but I don't think we offer a generic solution until we address specific use-cases. The solution can start as experimental/private APIs...

#16 in reply to: ↑ 15 @jonsurrell
10 months ago

@youknowriad I have some questions about the approach of adding new modules for the functionality that modules require. Let's look at an apiFetch module as an example folks are already requesting that exhibits challenges for a module.

The wp-api-fetch script relies on some imperative code to set it up for use in WordPress, that's something we'd need to address. There are likely other things we could improve in its module version.

What's the module version going to look like? Does it try to maintain the same API for the sake of familiarity? What does this mean for the @wordpress/api-fetch package that already includes module and commonjs builds? Will there be a new package for the modern api-fetch module version? Releasing a new major version with breaking changes would be a good way to move forward in the npm ecosystem, but does that imply a breaking change for the wp-api-fetch script? How would the script be maintained independently of the module version?

To be clear, I'm not opposed to the approach. I do like that it provides a way to separate more modern modules from legacy scripts and an opportunity to start with a clean slate, but I do want to explore the some of the difficult questions it raises.


I see more value personally (for a start) to be able to use modules within scripts (lazy load modules).

This may be sufficient for its own ticket. If we want to implement this I think the work will be separate from allowing dependence in the other direction.

#17 @jonsurrell
10 months ago

I've come to agree with @youknowriad that we should introduce modules progressively as needed without migrating whatever we have in scripts to modules.

I'm currently working on a proof-of-concept to migrate apiFetch functionality to a module. I'd like to keep its API mostly the same so folks don't have to learn a new API. Building and maintaining modules will bring with it another set of challenges.

apiFetch is also a nice module to explore because it has imperative configuration in WordPress that's currently handled by wp_add_inline_script. I believe modules will need an interface to pass configuration from the server to the client, but rather than the imperative style scripts currently use, modules should be able to read data on demand. PR#6433 (work in progress at the moment) contains an approach that introduces a filter scriptmoduledata_{$module_id} that can be used to attach data to an object that will be JSON serialized in the DOM for modules to read.

This ticket was mentioned in Slack in #core by nhrrob. View the logs.


9 months ago

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


9 months ago
#19

Add the print_script_module_data function to the WP_Script_Modules class.

Register hooks to call this function on wp_footer and admin_print_footer_scripts.

See the Make/Core Proposal and the PR in Gutenberg.

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

#20 @oglekler
9 months ago

  • Milestone changed from 6.6 to 6.7

We have 3 hours until the commit freeze before Beta 1, and it looks like this ticket will not be in Trunk in time. I am rescheduling it to the next milestone. If you are hard at work and about to merge, return it and finish the work.

#21 in reply to: ↑ 15 @jonsurrell
8 months ago

Replying to youknowriad:

I see more value personally (for a start) to be able to use modules within scripts (lazy load modules). For example a very concrete use-case I always bring is to be able to load a "codemirror" module in the editor for the code view for instance. I'd like editPost to be able to do import( '@wordpress/codemirror' ) when needed.

I've created #61500 for allowing Scripts to depend on Script Modules.

@jonsurrell commented on PR #6682:


8 months ago
#22

Suggested commit message:

Script Modules: Add new API to embed server data in HTML.

Add a new filter "script_module_data_{$module_id}" to associate data
with a Script Module. For example:

add_filter(
	'script_module_data_MyScriptModuleID',
	function ( array $data ): array {
		$data['script-needs-this-data'] = 'ok';
		return $data;
	}
);


If the Script Module is included in the page, enqueued or as a
dependency, the associated data will be JSON-encoded and embedded in the
HTML in a <script type="application/json"> tag with an ID of the form
"wp-script-module-data-{$module_id}" allowing the Script Module to
access the data on the client.

See the original proposal: https://make.wordpress.org/core/2024/05/06/proposal-server-to-client-data-sharing-for-script-modules/

Part of #60647

Developed in https://github.com/WordPress/wordpress-develop/pull/6682

Props jonsurrell, cbravobernal, westonruter, gziolo, bernhard-reiter, youknowriad, sergiomdgomes, czapla.

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


5 months ago
#24

Rework the way that Script Modules are registered.

This is a companion to https://github.com/WordPress/gutenberg/pull/65460 that requires porting to Core. Namely, the block-library changes require registration with their updated script module IDs so that the blocks continue to work correctly. This change is safe to land before the block-library package is updated.

A change landed in Gutenberg anticipating this change. It's essential to name the core function wp_default_script_modules accordingly or update Gutenberg:

https://github.com/WordPress/gutenberg/blob/2632234b2bdd6ef8e8b89e7521d370cfa0041764/lib/client-assets.php#L652

This includes the necessary ports from https://github.com/WordPress/gutenberg/pull/65460.

## Why?

  • Registering Script Modules in several places is messy and difficult to maintain.
  • Script Modules dependency arrays and versions were manually maintained, which is error prone.
  • Some block library code included Gutenberg-specific functions and constants which is undesirable.

## Testing Instructions

Try out this PR. Interactivity script modules (@wordpress/interactivity and @wordpress/interactivity-router) should be served from the script-modules directory.
I've added and reverted a commit that builds with the expected block-library changes soon to be included from Gutenberg. That specific commit can be used to test the block-library for future proof.

Try enabling and disabling SCRIPT_DEBUG. It should control the use of minified or non-minified assets. @wordpress/interactivity should use its debug version of the script with SCRIPT_DEBUG enabled.

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

@jonsurrell commented on PR #7360:


5 months ago
#26

@gziolo I think CI will pass now, this should be ready for review.

@jonsurrell commented on PR #7360:


5 months ago
#27

There are some test failures, it looks like an easy fix from the newly deprecated function:

Unexpected deprecation notice for WP_Interactivity_API::register_script_modules.

@jonsurrell commented on PR #7360:


5 months ago
#28

There are some test failures, it looks like an easy fix from the newly deprecated function:

Unexpected deprecation notice for WP_Interactivity_API::register_script_modules.

@jonsurrell commented on PR #7360:


5 months ago
#29

There are some test failures, it looks like an easy fix from the newly deprecated function:

Unexpected deprecation notice for WP_Interactivity_API::register_script_modules.

@gziolo commented on PR #7360:


5 months ago
#30

There are some test failures, it looks like an easy fix from the newly deprecated function:

Unexpected deprecation notice for WP_Interactivity_API::register_script_modules.

Tests should no longer run these two deprecated methods.

@jonsurrell commented on PR #7360:


5 months ago
#31

@michalczaplinski @darerodz It would be great to test out the Interactivity API on this PR as well as the interactive blocks in the block library.

There's a reverted commit on this branch that can be re-applied to test with new block-library versions as well. The commit message is "Test with block-library assets from Gutenberg" (currently 404336b).

@jonsurrell commented on PR #7360:


5 months ago
#32

I've started preparing next steps in https://github.com/WordPress/wordpress-develop/pull/7405 to add the a11y script module.

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


5 months ago
#33

  • Keywords has-unit-tests added

@noisysocks commented on PR #7360:


5 months ago
#34

Nice work here, thanks. Quick reminder that deadline to commit this backport is 6.7 Beta 1 which is scheduled for 1 October.

#35 @gziolo
5 months ago

In 59083:

Build: Prepare for more Script Modules

This is a companion to https://github.com/WordPress/gutenberg/pull/65460 that requires syncing in WordPress Core. Namely, the block-library changes require registration with their updated script module IDs so that the blocks continue to work correctly.

They key improvement is script modules registration is handled in one central place, and a combined asset file is used to improve the performance by avoiding multiple disk operations for every individual file.

Props jonsurrell, gziolo, wildworks, noisysocks.
See #60647, #59462.

#37 @gziolo
5 months ago

For these reasons, I think it's not worth it to try to load scripts in modules. I think we'd better try having build scripts that creates a script and a module for some selected packages. And I can even see some differences between the script and the module version of something. For instance configuring the translations shouldn't be done in the same way to wp-i18n, we'd have to load contextual translations instead (lazy load them as needed rather than print them server side...)

We iterate on this ticket based on the plan outlined by @youknowriad. I have just landed all the preparatory work done by @jonsurrell to align how script modules are integrated with WordPress core to mirror the same handling as scripts which was iterated for many WP releases. The next step is exposing more packages with the following priorities:

  • @wordpress/a11y which is nearly ready for WP 6.7 - https://github.com/WordPress/wordpress-develop/pull/7405, it is the best one to verify as it can be used to refactor @wordpress/interactivity-router and we could use it for the Gallery block when enabling expand on click feature.
  • @wordpress/api-fetch - the most requested package.
  • @wordpress/hooks - that's the key WP feature.
  • @wordpress/i18n - #60234 depends on it.

There are some other nice to have some other vanilla JS packages that we could consider in the future:

  • @wordpress/date - it requires configuration on the server, useful only when it no longer depends on momentjs
  • @wordpress/is-shallow-equal
  • @wordpress/keycodes - not sure if loading a module for a constant is worth it
  • @wordpress/priority-queue
  • @wordpress/url - only WP specific utils, the same considerations as with @wordpress/keycodes
  • @wordpress/warning - that one could be used with @wordpress/interactivity package to fix https://github.com/WordPress/gutenberg/issues/61765

There might be more, all of them don't depend on React so that seems for the time being the leading indicator of whether a package is universal enough. However, it isn't something that is clear enough at this stage. Happy to discuss it later, but for now we would like to tackle the most requested listed as top priorities.
wordcount - might be too specific

@jonsurrell commented on PR #7405:


5 months ago
#38

@gziolo @darerodz @michalczaplinski this is ready for review.

It can't be tested with https://github.com/WordPress/wordpress-develop/pull/7304 at this time unless interactivity-router the package is modified in node_modules to modify this check.

#39 @jonsurrell
5 months ago

https://github.com/WordPress/wordpress-develop/pull/7405 is ready for review to add expose a @wordpress/a11y Core Script Module.

This ticket was mentioned in Slack in #core by sabernhardt. View the logs.


5 months ago

#42 @czapla
5 months ago

In 59089:

Script Loader: Add @wordpress/a11y as a Script Module.

The Script Module has the same API as the wp-a11y WP Script.

Key changes:

  • Add @wordpress/a11y to the list of Script and Module dual packages.
  • Update script-modules-packages.min.php to include the a11y module.
  • Modify WP_Script_Modules class to track and handle a11y module availability.
  • Add method to print required HTML markup for a11y speak() functionality.

See #60647.
Props jonsurrell, gziolo, czapla.

#45 @czapla
5 months ago

In 59097:

Interactivity API: Move interactivity-router i18n strings to Script Module data.

Moves the 'loading' and 'loaded' i18n strings for the interactivity-router to the script module data via the script_module_data_@wordpress/interactivity-router filter.

Key changes:

  • Add the filter_script_module_interactivity_router_data() method, hooked into the script_module_data_@wordpress/interactivity-router filter, to set the i18n data with the 'loading' and 'loaded' messages.
  • Rename the print_router_loading_and_screen_reader_markup() method to print_router_markup() and remove the screen reader markup from it because it's no longer needed.
  • Remove the loading and loaded strings from the core/router store state because they're no longer needed.
  • Initialize the core/router store with a minimal navigation object to prevent errors in the interactivity-router script module when the store is not properly initialized.
  • Update corresponding unit tests to reflect these changes.

This change ensures that the interactivity-router i18n messages are localized in a single place and removes the need to initialize them in the core/router store state.

Props jonsurrell, swissspidy, czapla.
See #60647.

This ticket was mentioned in Slack in #core by swissspidy. View the logs.


5 months ago

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


5 months ago
#48

Reapply changes from [59097] / https://github.com/WordPress/wordpress-develop/pull/7304.
Restore and deprecated the renamed method print_router_loading_and_screen_reader_markup.

Description and testing instructions are the same as on the original PR:

Accounts for changes to the @wordpress/interactivity-router module in:

Static localized strings are passed via script module data passing instead of in Interactivity API store state.
Redundant HTML used for aria-live regions is removed. This functionality is not handled by the @wordpress/a11y script module added in #7405.

## Testing

Testing this requires package updates from https://github.com/WordPress/gutenberg/pull/65539. It anticipates the package being released and updated in Core.

(Copied from https://github.com/WordPress/gutenberg/pull/65539)

This PR is difficult to test on its own because it requires the package to be updated as a Core dependency for proper testing. It does not affect Gutenberg behavior, only the package's behavior in Core. There are other ways of doing it but this works:

  • Get a checkout of Core from https://github.com/WordPress/wordpress-develop/pull/7304. This is the build that will be running.
  • Run npm ci in the Core checkout to install packages.
  • Build the package from this PR (npm run build:packages in Gutenberg).
  • Replace the @wordpress/interactivity-router package in Core's node_modules (node_modules/@wordpress/interactivity-router) with the package directory in Gutenberg (remove the directory and copy the directory from Gutenberg packages/interactivity-router).
  • In Core, run the npm build script you use, in my case npm run dev.

Then test out that the interactivity router a11y functionality is working. A good way to test is the query block with "force page reload" option disabled. Specifically, the aria-live regions of the page should be updated (with localized text if in non-English locale) announcing "Page loaded."

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

@jonsurrell commented on PR #7447:


5 months ago
#49

Here's the diff comparing this with the original commit from ad1fabe417 / [59097]:

  • src/wp-includes/interactivity-api/class-wp-interactivity-api.php

    diff --git before/src/wp-includes/interactivity-api/class-wp-interactivity-api.php after/src/wp-includes/interactivity-api/class-wp-interactivity-api.php
    index e4dec8e262..8ffd612b13 100644
    old new final class WP_Interactivity_API { 
    993993CSS;
    994994        }
    995995
     996        /**
     997         * Deprecated.
     998         *
     999         * @since 6.5.0
     1000         * @deprecated 6.7.0 Use {@see WP_Interactivity_API::print_router_markup} instead.
     1001         */
     1002        public function print_router_loading_and_screen_reader_markup() {
     1003                _deprecated_function( __METHOD__, '6.7.0', 'WP_Interactivity_API::print_router_markup' );
     1004        }
     1005
    9961006        /**
    9971007         * Outputs markup for the @wordpress/interactivity-router script module.
    9981008         *

@jonsurrell commented on PR #7447:


5 months ago
#50

@michalczaplinski This is ready to try again 🙂

@jonsurrell commented on PR #7304:


5 months ago
#51

This was reverted to avoid removing the public method.

New PR with these changes: https://github.com/WordPress/wordpress-develop/pull/7447

#52 @czapla
5 months ago

In 59130:

Interactivity API: Move interactivity-router i18n strings to Script Module data.

Moves the 'loading' and 'loaded' i18n strings for the interactivity-router to the script module data via the script_module_data_@wordpress/interactivity-router filter.

Key changes:

  • Add the filter_script_module_interactivity_router_data() method, hooked into the script_module_data_@wordpress/interactivity-router filter, to set the i18n data with the 'loading' and 'loaded' messages.
  • Rename the print_router_loading_and_screen_reader_markup() method to print_router_markup() and remove the screen reader markup from it because it's no longer needed.
  • Deprecate the print_router_loading_and_screen_reader_markup() method.
  • Remove the loading and loaded strings from the core/router store state because they're no longer needed.
  • Initialize the core/router store with a minimal navigation object to prevent errors in the interactivity-router script module when the store is not properly initialized.
  • Update corresponding unit tests to reflect these changes.

This change ensures that the interactivity-router i18n messages are localized in a single place and removes the need to initialize them in the core/router store state.

Props jonsurrell, swissspidy, czapla, gziolo.
See #60647.

#54 @davidbaumwald
5 months ago

@czapla @jonsurrell @gziolo What feature/enhancement work still remains for this in 6.7? Just want to make sure the remaining work of that type is completed in time for Beta 1, which is scheduled to release in a few hours.

#55 @czapla
5 months ago

@davidbaumwald The work for 6.7 is complete now. It mostly revolved around shipping @wordpress/a11y as a module and preparatory work for this.

Those changes were committed in:

#56 @davidbaumwald
5 months ago

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

@czapla Thanks!

#57 @gziolo
5 months ago

  • Keywords needs-dev-note added

I think it's fair to say that the mission is completed for the @wordpress/a11y package, which is now exposed as a regular script and as a script module. As part of the effort, all the necessary enhancements in the codebase were provided to make it simpler to expose more script modules based on existing WordPress packages. It's only the beginning of the process, as outlined in my comment: https://core.trac.wordpress.org/ticket/60647#comment:37. I'm wondering what would be the best strategy moving forward in terms of tracking progress. Should we open tickets for other packages we would like to expose as script modules?

#58 @jonsurrell
5 months ago

  • Milestone changed from 6.7 to 6.8
  • Resolution fixed deleted
  • Status changed from closed to reopened

As a larger effort (epic) this work is not finished as noted in this comment. I'll reopen this ticket with milestone 6.8 for now. If there's a different approach that should be taken I'm happy to adjust.

#59 @financialcalculators
5 months ago

Is the problem discussed in the below thread related to this ticket?

JavaScript Translations Not Working with ES6 Modules in WordPress

https://wordpress.org/support/topic/javascript-translations-not-working-with-es6-modules-in-wordpress/#post-18050453

Summary:

WordPress’ i18n foreign language translation functionality does not work as expected in JavaScript when a plugin utilizes ES6 modules. The translation data, which should be injected into the webpage, is removed after the script_loader_tag filter is applied. This behavior prevents JavaScript/.json translations from being displayed.

Last edited 5 months ago by financialcalculators (previous) (diff)

This ticket was mentioned in Slack in #core by audrasjb. View the logs.


4 days ago

#61 @audrasjb
4 days ago

  • Keywords close added

Hello there, this ticket was reviewed during today's bug scrub. Umbrella tickets that stay opened across multiple release are hard to track, so it would be best to close it as fixed and open new, smaller tickets to handle further changesets.

Note: See TracTickets for help on using tickets.