Migrate to `import { initAll }` from ES module bundle #3833

colinrotherham · 2023-06-20T07:16:18Z

This PR follows our switch to ES modules:

It changes our review app to use ES module import not window.GOVUKFrontend.* for #3508

Before

We include our JavaScript as an IIFE via <script type="module">

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

After

We import our JavaScript as an ES module bundle via <script type="module">

<script type="module" src="/javascripts/all.bundle.min.mjs"></script>
<script type="module">
  import { initAll } from '/javascripts/all.bundle.min.mjs'
  initAll()
</script>

Whilst the first <script> is optional we've kept it for the browser preload scanner (see comment)

colinrotherham · 2023-06-20T07:17:04Z

packages/govuk-frontend-review/src/views/layouts/_generic.njk

@@ -4,6 +4,10 @@
  <link rel="stylesheet" href="/stylesheets/app.min.css">

  {% block styles %}{% endblock %}
+
+  <script type="importmap">
+    { "imports": { "govuk-frontend": "/javascripts/all.bundle.min.mjs" } }


An import map has been added to resolve govuk-frontend to /javascripts/all.bundle.min.mjs when run inside Puppeteer page.evaluate() like this:

await page.evaluate(async (exportName) => { const { CharacterCount } = await import('govuk-frontend') const $module = document.querySelector('[data-module]') const component = new CharacterCount($module) component.init() })

It also ensures that our on-page evaluated JavaScript can be type checked

question Is that importmap used outside of the test boilerplate? If not, I'd suggest moving it to that template specifically (even if it lives at the top of the <body> tag) as it's a feature to help with the tests 😊

It's used by renderAndInitialise() via /tests/boilerplate

govuk-frontend/shared/helpers/puppeteer.js

Line 101 in bf728fc

const namespace = await import('govuk-frontend')

(Few other tests also use boilerplate)

But all the globals tests (now "exports" rather than globals) use the home page /

govuk-frontend/packages/govuk-frontend/src/govuk/components/globals.test.mjs

Line 52 in bf728fc

const namespace = await import('govuk-frontend')

govuk-frontend/packages/govuk-frontend/src/govuk/components/globals.test.mjs

Line 20 in bf728fc

const namespace = await import('govuk-frontend')

govuk-frontend/packages/govuk-frontend/src/govuk/components/globals.test.mjs

Line 12 in bf728fc

Object.keys(await import('govuk-frontend'))

Let me just double check they'd all work from the boilerplate too

@romaricpascal Yeah all fine, I've moved it to the boilerplate only

colinrotherham · 2023-06-20T07:20:38Z

packages/govuk-frontend-review/src/views/layouts/_generic.njk

@@ -15,6 +19,9 @@
 {% set mainClasses = 'govuk-main-wrapper--auto-spacing' %}

 {% block bodyEnd %}
-  <script type="module" src="/javascripts/all.min.js"></script>
-  <script type="module">window.GOVUKFrontend.initAll()</script>
+  <script type="module" src="/javascripts/all.bundle.min.mjs"></script>


Keeping the <script> tag allows browser preload scanners to download our ES module bundle early

It's optional and can be replaced with modulepreload in future (Safari 17+)

<link rel="modulepreload" href="/javascripts/all.bundle.min.mjs">

Note: See browser support for <link rel="modulepreload">

colinrotherham · 2023-06-22T13:40:00Z

@romaricpascal We're 100% "double download" free in

Safari 11.1
Safari 12.1

☝️ Both browsers show concurrent download alongside the CSS

Additional checks in Internet Explorer 11 and Safari 10.1 show components without JavaScript

These tests appear to use dynamic `import()` but really they’re transformed to `require()` by Jest to clear the “require cache” between test runs Since we’d like to start using _real_ dynamic `import()` in our Puppeteer tests (to import `govuk-frontend` as an ES module) the task arguments must be converted back to CommonJS See: https://jestjs.io/docs/ecmascript-modules

This fixes an issue running `import()` via Puppeteer by turning off dynamic import `require()` transforms that use Babel runtime helper

An import map has been added to resolve `govuk-frontend` to `/javascripts/all.bundle.min.mjs` when run inside Puppeteer `page.evaluate()`

romaricpascal

Nice to totally take advantage of the ES modules for importing our API 🥳

I think the review app and its tests are in a bit of a wonky state, though, as it neither follows:

an import version of https://frontend.design-system.service.gov.uk/importing-css-assets-and-javascript/#add-the-javascript-file-to-your-html, where we would serve node_modules/govuk-frontend/dist/govuk/all.mjs without bundling
https://frontend.design-system.service.gov.uk/importing-css-assets-and-javascript/#import-javascript-using-a-bundler, where the bundled file would be the one initialising the components

So we're not quite testing that GOV.UK Frontend does what it's meant to in our tests, more that our review app bundle does what GOV.UK Frontend is meant to do (in a closer way than before so that's a win, though 🥳 ).

Let's merge this step forward and I'll add a note on #3508 to update how the review app serves/bundles govuk-frontend so we do test govuk-frontend and not the review app bundle (I'd say in favour of 1 so we actually test govuk-frontend through the review app).

colinrotherham added documentation User requests new documentation or improvements to existing documentation javascript labels Jun 20, 2023

govuk-design-system-ci temporarily deployed to govuk-frontend-pr-3833 June 20, 2023 07:16 Inactive

colinrotherham commented Jun 20, 2023

View reviewed changes

colinrotherham linked an issue Jun 20, 2023 that may be closed by this pull request

Update review app to use new JavaScript loading strategy #3508

Closed

colinrotherham mentioned this pull request Jun 20, 2023

Update review app to use new JavaScript loading strategy #3508

Closed

colinrotherham force-pushed the es-module-import branch from a67967a to 091afe1 Compare June 20, 2023 12:44

govuk-design-system-ci temporarily deployed to govuk-frontend-pr-3833 June 20, 2023 12:44 Inactive

colinrotherham force-pushed the es-module-import branch from 091afe1 to bf728fc Compare June 20, 2023 14:51

govuk-design-system-ci temporarily deployed to govuk-frontend-pr-3833 June 20, 2023 14:51 Inactive

colinrotherham added 2 commits June 22, 2023 17:42

Prevent Babel converting dynamic import() to require()

e5399f9

This fixes an issue running `import()` via Puppeteer by turning off dynamic import `require()` transforms that use Babel runtime helper

colinrotherham force-pushed the es-module-import branch from bf728fc to 1392b23 Compare June 22, 2023 16:43

govuk-design-system-ci temporarily deployed to govuk-frontend-pr-3833 June 22, 2023 16:44 Inactive

colinrotherham added 3 commits June 22, 2023 17:49

Update test boilerplate to use component preview layout

129d30b

Update test boilerplate to use component preview layout

dadba0f

Update Puppeteer tests to use ES modules not window globals

f1a0637

An import map has been added to resolve `govuk-frontend` to `/javascripts/all.bundle.min.mjs` when run inside Puppeteer `page.evaluate()`

colinrotherham force-pushed the es-module-import branch from 1392b23 to f1a0637 Compare June 22, 2023 16:50

govuk-design-system-ci temporarily deployed to govuk-frontend-pr-3833 June 22, 2023 16:50 Inactive

romaricpascal approved these changes Jun 22, 2023

View reviewed changes

colinrotherham merged commit 51461de into main Jun 22, 2023

colinrotherham deleted the es-module-import branch June 22, 2023 17:07

colinrotherham mentioned this pull request Jun 23, 2023

Fix Browsersync review app watch extension #3847

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Migrate to `import { initAll }` from ES module bundle #3833

Migrate to `import { initAll }` from ES module bundle #3833

colinrotherham commented Jun 20, 2023 •

edited

Loading

colinrotherham Jun 20, 2023 •

edited

Loading

romaricpascal Jun 22, 2023

colinrotherham Jun 22, 2023

colinrotherham Jun 22, 2023

colinrotherham Jun 20, 2023 •

edited

Loading

colinrotherham commented Jun 22, 2023 •

edited

Loading

romaricpascal left a comment

Migrate to import { initAll } from ES module bundle #3833

Migrate to import { initAll } from ES module bundle #3833

Conversation

colinrotherham commented Jun 20, 2023 • edited Loading

Before

After

colinrotherham Jun 20, 2023 • edited Loading

Choose a reason for hiding this comment

romaricpascal Jun 22, 2023

Choose a reason for hiding this comment

colinrotherham Jun 22, 2023

Choose a reason for hiding this comment

colinrotherham Jun 22, 2023

Choose a reason for hiding this comment

colinrotherham Jun 20, 2023 • edited Loading

Choose a reason for hiding this comment

colinrotherham commented Jun 22, 2023 • edited Loading

romaricpascal left a comment

Choose a reason for hiding this comment

Migrate to `import { initAll }` from ES module bundle #3833

Migrate to `import { initAll }` from ES module bundle #3833

colinrotherham commented Jun 20, 2023 •

edited

Loading

colinrotherham Jun 20, 2023 •

edited

Loading

colinrotherham Jun 20, 2023 •

edited

Loading

colinrotherham commented Jun 22, 2023 •

edited

Loading