DRAFT: PR for enhancing WP_Scripts with a loading strategy #54

joemcgill · 2023-04-03T18:03:02Z

⚠️ IMPORTANT This is a draft PR so we can review the diffs between our feature branch and trunk. DO NOT MERGE.

Co-authored-by: Joe McGill <801097+joemcgill@users.noreply.github.com>

…tps://github.com/10up/wordpress-develop into enhancement/get_eligible_loading_strategy-ML2-12

…enhancement/do_item-ML2-14

Co-authored-by: Simon Dowdles <72872375+10upsimon@users.noreply.github.com>

…enhancement/do_item-ML2-14

Co-authored-by: Simon Dowdles <72872375+10upsimon@users.noreply.github.com>

Co-authored-by: Joe McGill <801097+joemcgill@users.noreply.github.com>

…ript test function to accommodate script output changes as a result of core changes

…ease future lookups

…tion is true

src/wp-includes/class-wp-scripts.php

… output handling

…unction signature

src/wp-content/themes/twentytwentyone/functions.php

src/wp-includes/blocks.php

westonruter · 2023-04-26T21:01:49Z

Question: Should this PR also include cases where these new loading strategies are leveraged in core? Or is the thought that the PR be kept purely for the API changes and then to have separate PRs opened that make use of it?

felixarntz · 2023-04-26T23:06:23Z

@westonruter The latter, once the API is in core, we can start using it in a few places in core as well as the default themes.

src/wp-includes/script-loader.php

westonruter · 2023-04-27T20:06:06Z

Update: Comment moved to WordPress#4391 (comment).

When reviewing this PR, I was noticing how methods in WP_Scripts construct a lot of <script> tags manually with string concatenation, printf, variable interpolation, and calls to esc_attr()/esc_url(). Since WordPress 5.7, there have been dedicated helper functions (wp_get_inline_script_tag() and wp_get_script_tag()) which do all of the heavy lifting, as well as adding features like allowing the attributes to be filterable (for the sake of CSP). I've opened a draft PR (#58) for this branch which proposes using them. If desired, I can fix up the tests.

@adamsilverstein Do you know why these functions weren't utilized in WP_Scripts when they were introduced? I seem to recall this was punted for a later effort, but I can't find that info off hand.

…e the use of eval()

…ion body in script-loader.php

…cases

src/wp-includes/script-loader.php

joemcgill

Small spacing nitpicks

src/wp-includes/script-loader.php

tests/phpunit/tests/dependencies/scripts.php

…ding standards

10upsimon · 2023-04-27T20:50:35Z

Small spacing nitpicks

Resolved in recent commit @joemcgill

joemcgill · 2023-04-27T21:16:46Z

@10upsimon I believe we've wrapped up all blocking feedback on this draft PR at this point. When you're ready, please feel free to open a new PR to merge this branch into the trunk branch of the WordPress repo instead of this fork, and then I'll close this PR. We can pick up conversations about any additional enhancements we want to make (like #58) in the new PR that you open.

joemcgill · 2023-04-27T22:23:27Z

Closing this in favor of WordPress#4391, but we can still reference the open conversations and pick them up on the new PR opened against the WordPress repo.

kt-12 and others added 30 commits February 26, 2023 04:43

restore default values

5795b4c

phpcs fixes

0ebfaa5

Update text

f6b0483

Co-authored-by: Joe McGill <801097+joemcgill@users.noreply.github.com>

Readability

7b36d21

Co-authored-by: Joe McGill <801097+joemcgill@users.noreply.github.com>

consistent with other docblock descriptions

f71319c

Co-authored-by: Joe McGill <801097+joemcgill@users.noreply.github.com>

casting the result

624e797

Co-authored-by: Joe McGill <801097+joemcgill@users.noreply.github.com>

wordpress coding standards

b7334b7

Merge branch 'enhancement/get_eligible_loading_strategy-ML2-12' of ht…

2cb01be

…tps://github.com/10up/wordpress-develop into enhancement/get_eligible_loading_strategy-ML2-12

restore test case

5305f1c

add temporary ticket id

073684a

fix test bug

5e343b8

restore test case

b56c0e1

Merge branch 'enhancement/get_eligible_loading_strategy-ML2-12' into …

9caf8bb

…enhancement/do_item-ML2-14

temporary ticket added for testing

b70ee6c

Merge branch 'enhancement/get_eligible_loading_strategy-ML2-12' into …

dbf6da9

…enhancement/do_item-ML2-14

remove extra line

2deadcf

remove after dependency function

fcaa62e

Merge branch 'enhancement/get_eligible_loading_strategy-ML2-12' into …

150ba8b

…enhancement/do_item-ML2-14

fix doc block text

d578e50

Co-authored-by: Simon Dowdles <72872375+10upsimon@users.noreply.github.com>

Correct grammar

5262bf7

Co-authored-by: Simon Dowdles <72872375+10upsimon@users.noreply.github.com>

Fix doc block string.

2970f1a

Co-authored-by: Simon Dowdles <72872375+10upsimon@users.noreply.github.com>

change variable name

28af60d

doc block changes, better word

f746dfd

Merge branch 'enhancement/get_eligible_loading_strategy-ML2-12' into …

00a50d1

…enhancement/do_item-ML2-14

spacing issue.

d4bbaff

Co-authored-by: Simon Dowdles <72872375+10upsimon@users.noreply.github.com>

update placeholder with numbered placeholder

01e1d4f

Co-authored-by: Joe McGill <801097+joemcgill@users.noreply.github.com>

Change ticket id to trac id

a367951

Switching from numbered to normal substitution

51e3e41

Breaking test into multiple test functions

62cbf60

breaking test to dataprovider

0f80794

10upsimon added 3 commits April 26, 2023 11:24

Updates to test_non_standalone_after_inline_script_with_defer_main_sc…

9d0e68c

…ript test function to accommodate script output changes as a result of core changes

Added private var to store a handles dependents against, in order to …

3044aa3

…ease future lookups

Cleaner output handling within print_inline_script when display condi…

3ca847e

…tion is true

10upsimon reviewed Apr 26, 2023

View reviewed changes

src/wp-includes/class-wp-scripts.php Show resolved Hide resolved

10upsimon added 3 commits April 26, 2023 14:19

Cleanup/optimzation of code within do_item function for inline script…

edd3d52

… output handling

Docblock wording change

59324a8

Update applicable core calls to wp_register_script() to reflect new f…

6ab9511

…unction signature

10upsimon reviewed Apr 26, 2023

View reviewed changes

src/wp-content/themes/twentytwentyone/functions.php Show resolved Hide resolved

10upsimon reviewed Apr 26, 2023

View reviewed changes

src/wp-content/themes/twentytwentyone/functions.php Show resolved Hide resolved

10upsimon reviewed Apr 26, 2023

View reviewed changes

src/wp-includes/blocks.php Show resolved Hide resolved

Fix formatting issue causing PHPCS failure

5a94bfa

westonruter reviewed Apr 27, 2023

View reviewed changes

src/wp-includes/script-loader.php Outdated Show resolved Hide resolved

westonruter mentioned this pull request Apr 27, 2023

Eliminate manual construction of script tags #58

Closed

1 task

10upsimon added 4 commits April 27, 2023 22:11

Rewrite function body for wpLoadAfterScripts to be ES5 and not requir…

0e120b7

…e the use of eval()

Updated function body for wpLoadAfterScripts to reflect updated funct…

da5668f

…ion body in script-loader.php

Update js formatting within wpLoadAfterScripts function

9c3ded2

Update js formatting within wpLoadAfterScripts function used in test …

3d9d0d8

…cases

10upsimon reviewed Apr 27, 2023

View reviewed changes

src/wp-includes/script-loader.php Outdated Show resolved Hide resolved

joemcgill commented Apr 27, 2023

View reviewed changes

src/wp-includes/script-loader.php Outdated Show resolved Hide resolved

tests/phpunit/tests/dependencies/scripts.php Outdated Show resolved Hide resolved

Updated formatting of wpLoadAfterScripts js function to satisfy WP co…

430e5b2

…ding standards

10upsimon approved these changes Apr 27, 2023

View reviewed changes

joemcgill closed this Apr 27, 2023

westonruter mentioned this pull request May 5, 2023

Enhancing the WP_Scripts API With a Loading Strategy WordPress/wordpress-develop#4391

Closed

westonruter mentioned this pull request May 31, 2023

Add delayed inline before scripts and remove standalone inline scripts #67

Merged

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

DRAFT: PR for enhancing WP_Scripts with a loading strategy #54

DRAFT: PR for enhancing WP_Scripts with a loading strategy #54

joemcgill commented Apr 3, 2023

westonruter commented Apr 26, 2023

felixarntz commented Apr 26, 2023

westonruter commented Apr 27, 2023 •

edited

Loading

joemcgill left a comment

10upsimon commented Apr 27, 2023

joemcgill commented Apr 27, 2023

joemcgill commented Apr 27, 2023

DRAFT: PR for enhancing WP_Scripts with a loading strategy #54

DRAFT: PR for enhancing WP_Scripts with a loading strategy #54

Conversation

joemcgill commented Apr 3, 2023

westonruter commented Apr 26, 2023

felixarntz commented Apr 26, 2023

westonruter commented Apr 27, 2023 • edited Loading

joemcgill left a comment

Choose a reason for hiding this comment

10upsimon commented Apr 27, 2023

joemcgill commented Apr 27, 2023

joemcgill commented Apr 27, 2023

westonruter commented Apr 27, 2023 •

edited

Loading