Respond to Rollup warning for i18n.mjs

[36degrees]

What

This notice, which we think is coming from Rollup, currently appears in the test output:

Entry module “src/govuk/i18n.mjs” is implicitly using “default” export mode, which means for CommonJS output that its default export is assigned to “module.exports”. For many tools, such CommonJS output will not be interchangeable with the original ES module. If this is intended, explicitly set “output.exports” to either “auto” or “default”, otherwise you might want to consider changing the signature of “src/govuk/i18n.mjs” to use named exports only.

Understand whether there are implications for consumers of GOV.UK Frontend and whether we need to make any changes.

If we don't need to make any changes, see if there's a way to silence the warning.

Why

src/govuk/i18n.mjs is a new file which has not yet been included in a release of GOV.UK Frontend.

We want to make sure that the way that we've written i18n.mjs will not impact consumers of GOV.UK Frontend once the next release has gone out.

Who needs to work on this

Developers

Who needs to review this

Developers

Done when

• We understand the implications of the warning
• We've made any changes required before shipping the next release
• The warning no longer appears in the test output (unless it's not possible to silence it)

[romaricpascal]

Maybe using a named export would make the warning disappear, as it seems to complain about the export being a default one. If that's the case, that might be a cheap, quick fix for the warning 😄

[colinrotherham]

I'd prefer named exports too as it can also avoid compatibility problems with bundlers/compilers having to support a "synthetic default". Likely why Rollup is logging the issue

Two questions:

1. Any objections to switching to named imports?
2. Were we supposed to export I18n via all.js and all.mjs or is it private?

Before all.mjs
We import the default component, giving it a new name “Accordion”

import Accordion from './components/accordion/accordion.mjs'
export { Accordion }

After all.mjs
We import the already-named “Accordion” component

import { Accordion } from './components/accordion/accordion.mjs'
export { Accordion }

Subtle difference, but the “After” version will also let IDEs keep track of when things get renamed

[romaricpascal]

Won’t moving to the second version be a breaking change if people are already importing single ESM components?

[colinrotherham]

@romaricpascal For compatibility reasons perhaps we keep the default export (alongside the named one)?

But, do we actually support importing components directly like these examples?

Require/Import via filename default export

This is the current (working) approach to require/import components directly

// CommonJS modules
const Button = require('govuk-frontend/govuk/components/button/button')

// ECMAScript modules
import Button from 'govuk-frontend/govuk-esm/components/button/button.mjs'

// Both via dynamic import
const { default: Button } = await import('govuk-frontend/govuk-esm/components/button/button.mjs')

Require/Import via filename named export

This is the suggested Rollup approach but it's possible to support an export default fallback too

// CommonJS modules
const { Button } = require('govuk-frontend/govuk/components/button/button')

// ECMAScript modules
import { Button } from 'govuk-frontend/govuk-esm/components/button/button.mjs'

// Both via dynamic import
const { Button } = await import('govuk-frontend/govuk-esm/components/button/button.mjs')

Advantages: Linting and "IntelliSense" style tools will know when components get replaced, moved or renamed. Editors and IDEs with "refactor" functionality can rename all references when an import changes

Require/Import via govuk-frontend package exports

But really this may be the "public API" we'd expect people to use:

// CommonJS modules
const { Button } = require('govuk-frontend')

// ECMAScript modules
import { Button } from 'govuk-frontend'

// Both via dynamic import
const { Button } = await import('govuk-frontend')

☝️ @romaricpascal It's this last example that will continue to work (even without an export default fallback) thanks to the named exports we're already using in all.mjs and all.js

[colinrotherham]

If we want to avoid breaking "via filename named export" here's the compatibility suggestion:

Single default export only (current)
Currently Accordion isn't exported by name

function Accordion ($module, config) {} // Not exported
export default Accordion

Both named and default export (suggested)
But we could ensure Accordion keeps its default export

export function Accordion ($module, config) {} // Exported
export default Accordion

It's not really recommended (ESLint import plugin has a rule for no-named-as-default) but solves a temporary problem

[romaricpascal]

Cheers for the thorough description 😄 On my side, I'd be happy to move towards named for everything, but that's a decision that'll need a wider discussion I think.

If we go for it though, having both named and not named while we wait for our next breaking release doesn't sounds like too bad a compromise. That's a neat idea 🙌🏻

[colinrotherham]

Thanks @romaricpascal, there's one other potential issue as we use UMD bundles

I can't see the "interop" __esModule: true in the output

Questions

1. Does our pinned version of Rollup support __esModule?
2. Does Rollup support __esModule for UMD (see Babel's approach)
3. Should we identify bundlers that don't support the "interop" __esModule: true flag?

What does __esModule: true do?

The __esModule: true flag ensures (in most places) that ESM-to-XXX bundled code can still be imported as if it was an ECMAScript module. You can see where it's configured by default for Babel, Rollup, TypeScript and webpack

Ignoring UMD for this example, but something like:

Input

ECMAScript module via accordion.mjs

export function Accordion ($module, config) {} // Exported
export default Accordion

Output

CommonJS module via accordion.js

function Accordion ($module, config) {}

exports.__esModule = true
exports.Accordion = Accordion
exports.default = Accordion

[romaricpascal]

Cheers for demistifying __esModule, had never gone in depth on it, only knew it existed 😄

If I understood correctly what it does, wouldn't the situation only be problematic if someone was trying to directly import a CommonJS component from an ES module as such:

// Note that they're importing from `govuk` folder, not `govuk-esm`
import {Accordion} from 'govuk-frontend/govuk/components/accordion/accordion.js'

The other scenarios for importing, like from 'govuk-frontend' or `from 'govuk-frontend/govuk-esm/...' should be covered by the fact that we have a "module" (for bundlers) and "exports" (for Node and Webpack – maybe other modern bundlers – I believe).

In that case, shouldn't we not worry about it too much and guide them to import from the govuk-esm package in the docs?

Finally, to get back to the original issue, I'm wondering if we couldn't take it in two steps:

1. Make only I18n a named export for now to solve the rollup warning (if it does)
2. Have a wider discussion around module and package structure for 5.0 to see how we'd swap other modules to named export. It's worth noting that NodeJS's docs recommend using named exports for Dual CommonJS/ES module packages

[colinrotherham]

No problem @romaricpascal 😊

Yeah require/import from 'govuk-frontend' will continue to work just fine

Older bundlers/resolvers will stick with "main": "govuk/all.js" but newer ones follow the exports entry points:

Using ./govuk/all.js for const { initAll } = require('govuk-frontend')
Using ./govuk-esm/all.mjs for import { initAll } from 'govuk-frontend

Like your example, if we went ahead and shipped named exports (alongside default exports) we could see some problems with older tooling that didn't understand __esModule.

You might have seen this with other libraries that haven't got it quite right:

const example = require('oops').default
import { default as example } from 'oops'

We'll have to consider how UMD bundles affect this too

[36degrees]

Were we supposed to export I18n via all.js and all.mjs or is it private?

Given how much effort we've put into the localisation work, I think we should aim to make it public at some point so that it can be re-used in downstream components.

However, that doesn't have to be something we do right now, especially if we want to give ourselves the opportunity to make changes to its API without having to treat them as breaking changes.

Maybe this is something to discuss as a team? At the very least I'd like to check in with @querkmachine and see how it would feel about making it public now.

[querkmachine]

Maybe this is something to discuss as a team? At the very least I'd like to check in with @querkmachine and see how it would feel about making it public now.

I don't think there's anything stopping I18n from being public.

It's been built around the needs of our existing components and is definitely not the most fully-featured i18n script around (no genders, contextual grammar, etc.), so external users may run into the limitations of it harder than we have, but it does work (🤞).