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

Support script attributes like "async", "defer", "preload" #49

Open
kadamwhite opened this issue Nov 17, 2022 · 2 comments
Open

Support script attributes like "async", "defer", "preload" #49

kadamwhite opened this issue Nov 17, 2022 · 2 comments

Comments

@kadamwhite
Copy link
Collaborator

We should have a way of registering that a script should be rendered with an async attribute, or included in a preload list.

In another project using this asset loader, a team recently wrote a custom wrapper which used this logic to set the "defaults" for scripts:

  • If async is not explicitly set, and the script has no dependencies, then default to async loading.
  • If defer is not explicitly set, but the script has dependencies, then default to defer loading.

Then when registering the script the wrapper would add the relevant parameters:

		foreach ( [ 'async', 'defer' ] as $attr ) {
			if ( ! empty( $args[ $attr ] ) ) {
				wp_script_add_data( $args['handle'], $attr, true );
				break;
			}
		}

Then, filter the script based on that data:

function filter_script_loader_tag( string $tag, string $handle ) : string {
	foreach ( [ 'async', 'defer' ] as $attr ) {
		if ( ! wp_scripts()->get_data( $handle, $attr ) ) {
			continue;
		}

		// Prevent adding attribute when already added in #12009.
		if ( ! preg_match( ":\s$attr(=|>|\s):", $tag ) ) {
			$tag = preg_replace( ':(?=></script>):', " $attr", $tag, 1 );
		}

		// Only allow async or defer, not both.
		break;
	}

	return $tag;
}

The note about #12009 refers to this ticket to add async and defer handling to core wp_enqueue_script itself.

@mattheu
Copy link
Member

mattheu commented Nov 17, 2022

I wrote this on the other project… but I’m not 100% sure about it. It’s a bit “magic” and I’m not sure we can just default to async without unexpectedly breaking things.

Async scripts can execute before the DOM is ready, so need to be sure to handle that case in your JS If necessary. Enabling this in an update could well break existing code.

I also am unsure about another case. What if a defer loaded script depends on an async one. Will it wait for that to be available before execution? I think so, but Im not totally sure.

I think we should add async and defer loading as an option in asset loader but i probably would default to nothing (or possibly defer…)

@mattheu
Copy link
Member

mattheu commented Nov 17, 2022

Async scripts can execute before the DOM is ready, so need to be sure to handle that case in your JS If necessary. Enabling this in an update could well break existing code.

I guess this wouldn’t really be a problem if you loaded your script in the footer, even if it’s async 🤔

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants