Skip to content

Conversation

@devansh016
Copy link
Contributor

Summary

Fixes #1600

Relevant technical choices

@westonruter westonruter added [Type] Enhancement A suggestion for improvement of an existing feature [Plugin] Embed Optimizer Issues for the Embed Optimizer plugin (formerly Auto Sizes) labels Oct 17, 2024
return <<<JS
const lazyEmbedsScripts = document.querySelectorAll( 'script[type="application/vnd.embed-optimizer.javascript"]' );
const lazyEmbedScriptsByParents = new Map();
$script = file_get_contents( __DIR__ . '/embed-optimizer-lazy-load.js.js' ); // phpcs:ignore WordPress.WP.AlternativeFunctions.file_get_contents_file_get_contents -- It's a local filesystem path not a remote request.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this file needs to be prefixed with embed-optimizer-. Just lazy-load.js is sufficient. Also, I see you've duplicated the .js at the end.

Suggested change
$script = file_get_contents( __DIR__ . '/embed-optimizer-lazy-load.js.js' ); // phpcs:ignore WordPress.WP.AlternativeFunctions.file_get_contents_file_get_contents -- It's a local filesystem path not a remote request.
$script = file_get_contents( __DIR__ . '/lazy-load.js' ); // phpcs:ignore WordPress.WP.AlternativeFunctions.file_get_contents_file_get_contents -- It's a local filesystem path not a remote request.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Per below, let's rename this to just lazy-load.js.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the suggestion. I have made the requested changes.

@devansh016 devansh016 marked this pull request as ready for review October 17, 2024 17:18
@github-actions
Copy link

github-actions bot commented Oct 17, 2024

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: devansh016 <devansh2002@git.wordpress.org>
Co-authored-by: westonruter <westonruter@git.wordpress.org>
Co-authored-by: adamsilverstein <adamsilverstein@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

Copy link
Member

@adamsilverstein adamsilverstein left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks!

@westonruter westonruter added this to the embed-optimizer n.e.x.t milestone Oct 17, 2024
lazyEmbedParent
);
const embedScript =
/** @type {HTMLScriptElement} */ document.createElement(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added this type comment because PhpStorm was alerting me of this:

Argument type Element is not assignable to parameter type Node | string

I'm not sure why PhpStorm thinks that document.createElement() can return a string.

@westonruter westonruter merged commit 9e8b700 into WordPress:trunk Oct 17, 2024
17 of 18 checks passed
@westonruter westonruter changed the title Moves embed-optimizer-lazy-load script to js file Move embed-optimizer-lazy-load script to JS file Oct 21, 2024
@westonruter westonruter added no milestone PRs that do not have a defined milestone for release skip changelog PRs that should not be mentioned in changelogs and removed [Type] Enhancement A suggestion for improvement of an existing feature no milestone PRs that do not have a defined milestone for release labels Oct 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

[Plugin] Embed Optimizer Issues for the Embed Optimizer plugin (formerly Auto Sizes) skip changelog PRs that should not be mentioned in changelogs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

JavaScript module code inlined in PHP should be loaded from external JS files

3 participants