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

Migrate to use <script type="module"> #3769

Merged
merged 6 commits into from
Jun 8, 2023
Merged

Migrate to use <script type="module"> #3769

merged 6 commits into from
Jun 8, 2023

Conversation

colinrotherham
Copy link
Contributor

@colinrotherham colinrotherham commented Jun 8, 2023

This PR migrates the review app + webpack example to <script type="module"> for #3508

<script type="module" src="/javascripts/all.min.js"></script>
<script type="module">window.GOVUKFrontend.initAll()</script>

Components styled with .js-enabled

Once this PR merges there will be a disconnect between browsers that apply .js-enabled styling and those that can actually initialise <script type="module"> functionality but we’ll address that in a future commit

JavaScript load order

We ideally want GOV.UK Frontend to load before 3rd party scripts so that component initialisation isn't delayed

Since module scripts have defer loading behaviour we've added DOMContentLoaded to 3rd party scripts:

<script src="/vendor/iframe-resizer/iframeResizer.min.js" defer></script>
<script>
  document.addEventListener('DOMContentLoaded', function () {
    window.iFrameResize({}, '.js-component-preview')
  })
</script>

All packages now build into `dist` and this was left behind
@colinrotherham colinrotherham added documentation User requests new documentation or improvements to existing documentation javascript labels Jun 8, 2023
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-3769 June 8, 2023 08:43 Inactive
Once this PR merges there will be a disconnect between browsers that apply `.js-enabled` styling and those that can actually initialise `<script type="module">` functionality but we’ll address that in a future commit
Adding the `defer` attribute matches `<script type="module">` behaviour for non-module scripts

We now listen for `DOMContentLoaded` in inline scripts so the deferred script’s window globals are available

We also want to give priority to essential service code
We only bind to `pageshow` now (slight edit) since: ae75de1
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-3769 June 8, 2023 09:06 Inactive
@colinrotherham colinrotherham linked an issue Jun 8, 2023 that may be closed by this pull request
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-3769 June 8, 2023 09:32 Inactive
Copy link
Contributor

@36degrees 36degrees left a comment

Choose a reason for hiding this comment

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

Nice 🎉

@colinrotherham colinrotherham merged commit f087913 into main Jun 8, 2023
@colinrotherham colinrotherham deleted the module-script branch June 8, 2023 16:37
@colinrotherham
Copy link
Contributor Author

Linking the follow up PR to this one:

colinrotherham added a commit that referenced this pull request Jul 19, 2023
Migrate to use `<script type="module">`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation User requests new documentation or improvements to existing documentation javascript
Projects
Development

Successfully merging this pull request may close these issues.

Update review app to use new JavaScript loading strategy
3 participants