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

Implementation of load handlers to ensure that the execution order of inline scripts is retained, where applicable #20

Closed
1 task done
10upsimon opened this issue Feb 9, 2023 · 3 comments · Fixed by #50
Assignees
Labels
30 Estimate of 30 points Script Loading Strategy Issues relating to the Script Loading Strategy (WPP) type:enhancement New feature or request.
Milestone

Comments

@10upsimon
Copy link

10upsimon commented Feb 9, 2023

Description

Given that inline scripts need to retain their execution order even when their parent/main scripts contain a loading strategy such as defer or async, these scripts will need to be executed as specific times as dictated by the execution order of said parent/main scripts.

For example:

  1. For an after inline script, it could wrapped with a load event listener on its parent/main script (when it is deferred), so that the code is only executed once its parent/main script is loaded, whenever that may be.
  2. For a before inline script(s), the underlying dependency chain becomes complicated to solve for, with onload handlers needing to be applied to the last dependency prior to the inline script in question, or even after if said last dependency has an after script associated with it. Due to the inherent complexity surrounding this, before scripts should be forced to have a blocking strategy. This also means that any dependencies that are deferred should too become blocking unless the before script is opted in as standalone (see issue 15).

For any after inline script, using a load wrapper approach could have a sequence of events that look something like the following:

  1. An inline script(s) is registered against a handle that is marked for deferred loading.
  2. The inline script is checked for having a $standalone parameter value of true or false. If true, no further action is required and the script is treated as a standalone blocking inline script. If false, however, said inline script is output in the footer, but instead of being output with the usual <script type="text/javascript"> tag, it is output in a form that will not immediately be executed as JavaScript by the browser, i.e <script type="text/template">. This script element may be given a sensible data attribute associating it with a script handle and identifying it for later conversion to JavaScript and execution thereof. It should be noted that simply converting the script type will not ensure automatic execution by the browser, and execution thereof will need to be triggered programatically.
  3. A new common WordPress core (blocking) script is enqueued as the final script in the footer. This script harvests all <script type="text/template"> elements containing data-wp-executes-after attributes (or similar, this is merely an example), and references them and the handles they rely on.
  4. Logic is added to add event listeners listening for load events of said deferred parent/main script(s). The callback itself will parse the applicable <script type="text/template"> elements, changing them to <script type="text/javascript"> and applying necessary logic that causes them to be executed and parsed as JavaScript by the browser.

Acceptance Criteria

  • Enhance the existing WP_Scripts class in areas that handle both main and inline script preparation and printing to ensure that necessary data attributes are added to main/parent script of inline scripts, so that once they load, the necessary inline script(s) can be executed.
  • Addition of custom data-wp-executes-after data attribute to applicable inline scripts who have been identified as having deferred main/parent scrips, requiring a specific execution order, specifically those rendering as <script type="text/template">.
    • The attribute value will store a reference to the main script after which it requires execution following the load event of said parent/main script. This should be in the form of a string, i.e data-wp-executes-after="script-handle-1"
  • Creation of a new common core script that loads prior to the output of any script that has both a loading strategy of defer and has an associated inline script(s) that are not opted in as standalone (see issue 15).
    1. Said script should parse the DOM for the presence of any <script type="text/template"> elements.
    2. Said script should reference these element(s) against the main script handles as indicated by the custom data-wp-executes-after attribute applied to said element(s).
    3. Said script should apply load handlers to applicable main/parent scripts, referencing the necessary inline script elements as harvested above in sub point 1.
    4. Said load handler/event listener should, upon execution, convert the necessary <script type="text/template"> DOM elements to <script type="text/javascript"> and manually execute them as JavaScript in the browser.

Unit Tests

  • Unit tests for all script/dependency logic reside in the Tests_Dependencies_Scripts class.
  • Unit tests will need to be added/amended to cater for the following outcomes:
    • Inline scripts having been identified as requiring deferred execution should contain <script type="text/template"> as opposed to <script type="text/javascript">
    • Inline scripts having been identified as requiring deferred execution should contain the new custom data-wp-executes-after attribute.
    • Said custom attribute shall contain the necessary main/parent script handle after which said inline script is intended to execute once main/parent script has loaded.
    • The new common/core script shall load prior to the output of any script that has both a loading strategy of defer and has an associated inline script(s) that are not opted in as standalone.
    • The new common/core script shall correctly identify all inline scripts with <script type="text/template">.
    • The new common/core script shall create load event handlers/callback function that correctly reference the inline script(s) and their associated main/parent script(s).
    • Said inline scripts of type <script type="text/template"> shall be converted to <script type="text/javascript">
    • Said inline scripts shall be executed by the browser with necessary logic once converted to <script type="text/javascript">

Designs

No response

Describe alternatives you've considered

No response

Code of Conduct

  • I agree to follow this project's Code of Conduct
@10upsimon 10upsimon converted this from a draft issue Feb 9, 2023
@10upsimon 10upsimon added type:enhancement New feature or request. Script Loading Strategy Issues relating to the Script Loading Strategy (WPP) 30 Estimate of 30 points labels Feb 9, 2023
@10upsimon 10upsimon added this to the Milestone 3 milestone Feb 9, 2023
@joemcgill
Copy link
Collaborator

This is all looking good to me.

I don't quite understand one point in the sub-bullet describing the attribute value for data-wp-executes-after that reads "or an array of handles". What is the use case for when an array of handles would be used?

It would be nice to confirm the execution of these behaviors with a new e2e test if possible, but that doesn't have to be a hard requirement of this issue.

@10upsimon
Copy link
Author

@joemcgill I believe I may have added this in error. My thinking at that time was around the ability for multiple inline scripts to be associated with the same main/parent script. That however would not influence the attribute on the inline script as it would only ever reference the single main/parent script.

I'll remove that reference to the array of handles and move this to To Do.

@10upsimon
Copy link
Author

@joemcgill I've made said update to remove the incorrect reference to "an array of handles" in AC point 2. @kt-12 moving this over to To Do.

@joemcgill joemcgill removed their assignment Feb 16, 2023
@kt-12 kt-12 moved this from To Do to In Progress in Enhancing the Scripts API with a loading strategy Mar 10, 2023
@kt-12 kt-12 linked a pull request Mar 11, 2023 that will close this issue
@kt-12 kt-12 moved this from In Progress to Code Review in Enhancing the Scripts API with a loading strategy Mar 16, 2023
@eclarke1 eclarke1 moved this from Code Review to In Progress in Enhancing the Scripts API with a loading strategy Mar 22, 2023
@kt-12 kt-12 moved this from In Progress to Code Review in Enhancing the Scripts API with a loading strategy Mar 23, 2023
@eclarke1 eclarke1 assigned 10upsimon and unassigned kt-12 Mar 27, 2023
@10upsimon 10upsimon moved this from Code Review to Approval in Enhancing the Scripts API with a loading strategy Mar 27, 2023
@kt-12 kt-12 moved this from Approval to In Progress in Enhancing the Scripts API with a loading strategy Mar 31, 2023
@kt-12 kt-12 moved this from In Progress to Code Review in Enhancing the Scripts API with a loading strategy Mar 31, 2023
@eclarke1 eclarke1 moved this from Code Review to Approval in Enhancing the Scripts API with a loading strategy Apr 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
30 Estimate of 30 points Script Loading Strategy Issues relating to the Script Loading Strategy (WPP) type:enhancement New feature or request.
Projects
No open projects
Development

Successfully merging a pull request may close this issue.

3 participants