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

Inline scripts/styles should have sources identified and core script/style dependencies should get enqueueing theme/plugin as source #4134

Closed
westonruter opened this issue Jan 19, 2020 · 2 comments · Fixed by #4135
Assignees
Labels
Milestone

Comments

@westonruter
Copy link
Member

westonruter commented Jan 19, 2020

Feature description

Related: #4133

If a theme enqueues a navigation.js that has jQuery as a dependency, currently this appears in the validated URL screen as “wp-includes” as the source. This is not very helpful. It would be helpful that if a theme or plugin enqueues a script or a style, that the original theme/plugin that enqueued it would be attributed as a source in some way. As an example, consider this demo plugin.

All of the scripts here should get “Enqueue assets with core dependencies” as the source, at least indirectly:

image

And for the stylesheets, the same goes for the circled stylesheets:

image

I believe that Origination already provides support here for this, so this should perhaps be deferred to the incorporation of that library.

Also: For each script/stylesheet, the asset which caused the dependency to be printed should be listed.

Additionally, inline styles added via wp_add_inline_style() do not get their source attributed.

Inline Scripts

Scripts added via wp_localize_script() and wp_add_inline_script() should all get properly identified. For this, see another example plugin.


Do not alter or remove anything below. The following sections will be managed by moderators only.

Acceptance criteria

Implementation brief

QA testing instructions

Demo

Changelog entry

@westonruter westonruter self-assigned this Jan 19, 2020
@westonruter westonruter added this to the v1.5 milestone Jan 19, 2020
@kmyram kmyram added the Size: M label Jan 21, 2020
@westonruter westonruter changed the title Core script and style dependencies should get enqueueing theme/plugin as source Inline scripts/styles should have sources identified and core script/style dependencies should get enqueueing theme/plugin as source Jan 23, 2020
@kienstra
Copy link
Contributor

I'll do 'dev QA' for this.

@kienstra kienstra self-assigned this Feb 21, 2020
@kienstra
Copy link
Contributor

Looks Good!

Hi @westonruter,
It's really nice to have the sources of dependencies.

As you mentioned in the testing instructions, this shows the correct plugin sources for:

Scripts

plugin-here

Styles

style-identified

Similar to the plugins you shared, here's what I used to test this:

// Enqueues a script and stylesheet.
add_action(
	'wp_enqueue_scripts',
	function() {
		$script_slug = 'amp-scratchpad-script';
		wp_enqueue_script(
			$script_slug,
			plugins_url( 'script.js', __FILE__ ),
			[ 'jquery' ],
			'0.1',
			true
		);

		wp_localize_script(
			$script_slug,
			'ampScratchpad',
			wp_json_encode( [ 'foo' => 'baz' ] )
		);

		wp_add_inline_script(
			$script_slug,
			'doSomethingHere()'
		);

		$style_slug = 'amp-scratchpad-style';
		wp_enqueue_style(
			$style_slug,
			plugins_url( 'style.css', __FILE__ ),
			[ 'wp-color-picker', 'site-icon' ],
			'0.1'
		);

		wp_add_inline_style(
			$style_slug,
			'body { background-color: #ffffff; }'
		);
	}
);

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

Successfully merging a pull request may close this issue.

3 participants