Skip to content
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

Reconsider approach of using script dependencies to opt-in a script for worker offloading #1468

Closed
westonruter opened this issue Aug 13, 2024 · 1 comment · Fixed by #1497
Assignees
Labels
[Plugin] Web Worker Offloading Issues for the Web Worker Offloading plugin. [Type] Enhancement A suggestion for improvement of an existing feature

Comments

@westonruter
Copy link
Member

Currently the way that a script is offloaded to a web worker is to add the web-worker-offloading script dependency:

add_action( 'wp_enqueue_scripts', function () {
    wp_enqueue_script(
  	  'non-critical-js',
  	  '/path/to/non-critical.js',
  	  array( 'web-worker-offloading' ), // Note the `web-worker-offloading` being added as a dep
  	  '1.0.0',
  	  true
    );
} );

To opt-in existing scripts:

add_action( 'wp_print_scripts', function () {
	$scripts = wp_scripts();
	$handle  = 'foo';
	if ( array_key_exists( $handle, $scripts->registered ) ) {
		$scripts->registered[ $handle ]->deps[] = 'web-worker-offloading';
	}
} );

However, I just realized a significant problem: if someone deactivates the Web Worker Offloading plugin, then the web-worker-offloading script won't be registered and so the script having it asa dependency won't get printed at all. This would then necessitate having to add conditional checks like the following before doing the above code:

if ( defined( 'WEB_WORKER_OFFLOADING_VERSION' ) ) {
    // Now include the web-worker-offloading dependency.
}

We could make this more resilient by taking a similar approach to what we did with the script loading strategies (async/defer). In fact, offloading to a web worker could itself be considered a type of loading strategy. With script loading strategies, we utilize the strategy key for script data. So for example an existing script can be opted-in to the async strategy:

wp_script_add_data( 'foo', 'strategy', 'async' );

We could do something similar for offloading to a worker, for example:

wp_script_add_data( 'foo', 'worker', true );

Then when scripts are being printed, we could check if any of the pending handles have a worker script data key with a true value, and if so, print the Partytown script as well as add the type="text/partytown" attribute to the script being printed.

@westonruter westonruter added [Type] Enhancement A suggestion for improvement of an existing feature [Plugin] Web Worker Offloading Issues for the Web Worker Offloading plugin. labels Aug 13, 2024
@thelovekesh thelovekesh self-assigned this Aug 25, 2024
@adamsilverstein
Copy link
Member

Good catch on the flaw in the current approach!

I agree with your suggested approach which is much cleaner and aligns with my suggestion on the original ticket.

@westonruter westonruter self-assigned this Sep 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Plugin] Web Worker Offloading Issues for the Web Worker Offloading plugin. [Type] Enhancement A suggestion for improvement of an existing feature
Projects
Status: Done 😃
Development

Successfully merging a pull request may close this issue.

3 participants