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

ES Modules can't be imported without a bundler #2633

Closed
Tracked by #2653 ...
colinrotherham opened this issue May 19, 2022 · 18 comments · Fixed by #2658
Closed
Tracked by #2653 ...

ES Modules can't be imported without a bundler #2633

colinrotherham opened this issue May 19, 2022 · 18 comments · Fixed by #2658
Assignees
Labels
🐛 bug Something isn't working the way it should (including incorrect wording in documentation) javascript
Milestone

Comments

@colinrotherham
Copy link
Contributor

colinrotherham commented May 19, 2022

Hello! 👋

Loved seeing the ES Modules support land yesterday 😊

Description of the issue

I'm unfortunately seeing an Error [ERR_PACKAGE_PATH_NOT_EXPORTED] thrown without a bundler

Steps to reproduce the issue

This happens with a plain native Node.js import from another *.mjs file

Current (see package.json)
Missing the main or default export

"exports": {
  "import": "./govuk-esm/all.mjs"
}

Fixed (see exports documentation)
Provided a main or default export for both import and require()

"exports": {
  ".": {
    "import": "./govuk-esm/all.mjs",
    "require": "./govuk/all.js"
  }
}

Actual vs expected behaviour

Node's resolver node:internal/modules/esm/resolve doesn't have enough info to import 'govuk-frontend'

Unlike the Import JavaScript using a bundler guidance regarding webpack, I can't teach Node.js to break the ESM rules.

With the fix above, I sadly hit another challenge. Node.js wouldn't attempt to resolve .mjs files as there's no package.json "type": "module" and extensions are mandatory for CommonJS and ESM compatibility 😬

Mandatory file extensions
A file extension must be provided when using the import keyword. Directory indexes (e.g. './startup/index.js') must also be fully specified.

This behavior matches how import behaves in browser environments, assuming a typically configured server.

I wasn't fully out of options though.

I tried the new ESM aware TypeScript compiler but it also had issues importing without extensions. Microsoft have added some good release notes on type in package.json and New Extensions plus challenges involved.

Hope these notes help, it's fab to see what's coming 🚀

@colinrotherham colinrotherham added awaiting triage Needs triaging by team 🐛 bug Something isn't working the way it should (including incorrect wording in documentation) labels May 19, 2022
@colinrotherham
Copy link
Contributor Author

Could be the same issue affecting Rollup mentioned here #711 (comment)

Looks like the fix is similarly needed for import.meta.resolve('govuk-frontend') and require.resolve('govuk-frontend') when trying to determine "Where is GOV.UK Frontend installed" server-side

Similarly when using webpack to output a polyfill-light ESM bundle using library: { type: 'module' } (alongside a bigger polyfill-heavy CommonJS bundle for older browsers) it also has issues resolving imports

(But this might be a side effect of our very ESM setup!)

@colinrotherham
Copy link
Contributor Author

Found the code that's causing us issues

We have a monorepo with a shared webpack.config.mjs. Some local packages need to know where govuk-frontend is installed (it could be local to the package, or hoisted to a higher node_modules directory) so we run:

import { createRequire } from 'module'

// Avoids Node.js --experimental-import-meta-resolve
const require = createRequire(import.meta.url)

// Locate package
const pkgGovukFrontend = dirname(require.resolve('govuk-frontend'))

This is where Node.js throws Error [ERR_PACKAGE_PATH_NOT_EXPORTED]

[webpack-cli] Error [ERR_PACKAGE_PATH_NOT_EXPORTED]: No "exports" main defined in /path/to/project/node_modules/govuk-frontend/package.json
    at new NodeError (node:internal/errors:372:5)
    at throwExportsNotFound (node:internal/modules/esm/resolve:472:9)
    at packageExportsResolve (node:internal/modules/esm/resolve:693:7)
    at resolveExports (node:internal/modules/cjs/loader:482:36)
    at Function.Module._findPath (node:internal/modules/cjs/loader:522:31)
    at Function.Module._resolveFilename (node:internal/modules/cjs/loader:919:27)
    at Function.resolve (node:internal/modules/cjs/helpers:108:19)
    at file:///path/to/project/packages/frontend-assets/webpack.config.mjs:9:42
    at ModuleJob.run (node:internal/modules/esm/module_job:198:25)
    at async Promise.all (index 0) {
  code: 'ERR_PACKAGE_PATH_NOT_EXPORTED'
}

@lhokktyn
Copy link

We're experiencing this too. No bundler being used, just using createRequire() to resolve some paths within the govuk-frontend modules so that we may pull specific files for our particular use cases.

As it happens we're also supporting ESM/CommonJS consumers in our own module and although we've taken a slightly different approach (transpiling to CommonJS for all consumers) we have found success with the following config:

{
  "main": "dist/module.js",
  "module": "dist/mjs/module.js",
  "exports": {
    "import": "./dist/mjs/esm-wrapper.js",
    "require": "./dist/module.js"
  },
  ...
}

Where the esm-wrapper.js is an ESM script that simply imports the CommonJS transpiled sources, but would just be govuk-esm/all.mjs in your case.

Adding "require": "./govuk/all.js" to the list of "exports" fixes the issue for us.

@querkmachine querkmachine added 🕔 days javascript and removed awaiting triage Needs triaging by team labels May 24, 2022
@vanitabarrett
Copy link
Contributor

Thanks for raising this issue @colinrotherham and @lhokktyn , this is really detailed and helpful. We've been working on a fix for this issue which sounds similar #2643 and wondered if you'd be interested in helping test if this fix also solves the issues you're seeing?

You can test this by installing our pre-release:

npm install --save "alphagov/govuk-frontend#b456fa9c"

@colinrotherham
Copy link
Contributor Author

colinrotherham commented May 25, 2022

Thanks @vanitabarrett I've given it a go

Brilliantly our webpack build can now locate govuk-frontend BUT it's opened up more issues 😔

Sass

I need to dig into this some more but our webpack sass-loader can no longer resolve your stylesheets

I've seen that sass-loader@9 and above supports package.json "exports" so probably needs another entry? https://github.com/webpack-contrib/sass-loader/releases/tag/v9.0.0

Perhaps without exports.sass it's resolving relative to exports.import

Error: Module build failed (from ./node_modules/sass-loader/dist/cjs.js):
SassError: Can't find stylesheet to import.
  ╷
8 │ @import "govuk-frontend/govuk/base";
  │         ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  ╵
  src/assets/stylesheets/application.scss 8:9  root stylesheet
    at Object.<anonymous> (/path/to/project/node_modules/css-loader/dist/cjs.js??ruleSet[1].rules[2].use[1]!/path/to/project/node_modules/postcss-loader/dist/cjs.js!/path/to/project/node_modules/sass-loader/dist/cjs.js??ruleSet[1].rules[2].use[3]!/path/to/project/packages/frontend-assets/src/assets/stylesheets/application.scss:1:7)
    at /path/to/project/node_modules/webpack/lib/javascript/JavascriptModulesPlugin.js:441:11
    at Hook.eval [as call] (eval at create (/path/to/project/node_modules/tapable/lib/HookCodeFactory.js:19:10), <anonymous>:7:1)
    at Hook.CALL_DELEGATE [as _call] (/path/to/project/node_modules/tapable/lib/Hook.js:14:14)
    at /path/to/project/node_modules/webpack/lib/Compilation.js:5053:39
    at tryRunOrWebpackError (/path/to/project/node_modules/webpack/lib/HookWebpackError.js:83:7)
    at __webpack_require_module__ (/path/to/project/node_modules/webpack/lib/Compilation.js:5051:12)
    at __webpack_require__ (/path/to/project/node_modules/webpack/lib/Compilation.js:5008:18)
    at /path/to/project/node_modules/webpack/lib/Compilation.js:5079:20
    at symbolIterator (/path/to/project/node_modules/neo-async/async.js:3485:9)

Edit: This minor fix works for @import "govuk-frontend" but not @import "govuk-frontend/govuk/base"

"exports": {
  "import": "./govuk-esm/all.mjs",
  "require": "./govuk/all.js",
  "sass": "./govuk/all.scss"
},

JavaScript

Import JavaScript using a bundler
If I follow the If you’re using Webpack guidance for a webpack rule:

{
  test: /\.mjs$/,
  type: 'javascript/auto',
  resolve: {
    fullySpecified: false
  }
}

We can't benefit from this workaround as we also add:

exclude: /node_modules/,

This is so our full list of loaders: source maps, TypeScript, Babel, Sass, PostCSS (autoprefixer, minifier) don't process and/or transform external code. We don't want to tamper with external code until optimisation time.

That said, the fix for us would be to add this everywhere instead:

// enable ESM cheat code for govuk-frontend
exclude: /node_modules\/(?!govuk-frontend)/,

Without it we see this error for every module:

ERROR in ./node_modules/govuk-frontend/govuk-esm/all.mjs 6:0-59
Module not found: Error: Can't resolve './components/checkboxes/checkboxes' in '/path/to/project/node_modules/govuk-frontend/govuk-esm'
Did you mean 'checkboxes.mjs'?
BREAKING CHANGE: The request './components/checkboxes/checkboxes' failed to resolve only because it was resolved as fully specified
(probably because the origin is strict EcmaScript Module, e. g. a module with javascript mimetype, a '*.mjs' file, or a '*.js' file where the package.json contains '"type": "module"').
The extension in the request is mandatory for it to be fully specified.
Add the extension to the request.
resolve './components/checkboxes/checkboxes' in '/path/to/project/node_modules/govuk-frontend/govuk-esm'
  using description file: /path/to/project/node_modules/govuk-frontend/package.json (relative path: ./govuk-esm)
    Field 'browser' doesn't contain a valid alias configuration
    using description file: /path/to/project/node_modules/govuk-frontend/package.json (relative path: ./govuk-esm/components/checkboxes/checkboxes)
      Field 'browser' doesn't contain a valid alias configuration
      /path/to/project/node_modules/govuk-frontend/govuk-esm/components/checkboxes/checkboxes doesn't exist

@colinrotherham
Copy link
Contributor Author

colinrotherham commented May 25, 2022

@vanitabarrett Here's a fix that works for Node.js (ESM via webpack resolve workaround) and sass-loader with { webpackImporter: true }

@import "govuk-frontend/govuk/base";
@import "govuk-frontend/govuk/core/all";
@import "govuk-frontend/govuk/objects/all";
"exports": {
  ".": {
    "sass": "./govuk/all.scss",
    "import": "./govuk-esm/all.mjs",
    "require": "./govuk/all.js"
  },
  "./govuk/": "./govuk/",
  "./govuk-esm/": "./govuk-esm/"
},

Although using dart-sass imports via { webpackImporter: false } needs some work

@vanitabarrett vanitabarrett self-assigned this May 27, 2022
@vanitabarrett vanitabarrett added this to Backlog 🗄 in Design System Sprint Board via automation May 27, 2022
@vanitabarrett
Copy link
Contributor

Thanks for looking into this @colinrotherham , the team really appreciates it! I've been doing a bit of testing to try and make sure I understand the solution myself. Am I right in saying "sass": "./govuk/all.scss" is something specific to Webpack (or at least, only really used by Webpack at the moment) and allows a user to do @import "govuk-frontend" to import all.scss?

For importing any other sass files, it looks like "./govuk/": "./govuk/" is key to that working but I'm not sure I fully understand what that's doing, especially as it's referencing itself. The docs aren't the best at explaining some of this stuff 😬 Does it just open up that directory/path to allow users to import directly?

@colinrotherham
Copy link
Contributor Author

colinrotherham commented May 27, 2022

Not sure who started the package.json "sass" field really!

I had a quick look and your "sass" field was added in June 2018 (by @NickColley ⭐️) but it's likely one of the early Compass/Grunt/Gulp node-sass { importer } plugins that used "sass" too

A bit later sass-loader for webpack added "sass" support in December 2018

There's also the original (~2015) Eyeglass importer from the Sass people:

Foundation
https://github.com/foundation/foundation-sites/blob/develop/package.json#L142

"eyeglass": {
  "name": "foundation",
  "sassDir": "scss",
  "needs": ">=0.8.0",
  "exports": false
},

Must have followed on from frameworks and pattern library tools adopting style?
(Made popular by Browserify + Parcelify, PostCSS too)

Package entry points

The docs aren't brilliant and other tooling won't necessarily support the full spec
https://nodejs.org/api/packages.html#package-entry-points

The webpack resolver appears to prefer "exports" so explains why excluding "sass" caused #2645

Shorthand

Using the shorthand (following the documentation) would give us:

"exports": {
  "import": "./govuk-esm/all.mjs",
  "require": "./govuk/all.js",
  "sass": "./govuk/all.scss",
  "style": "./dist/govuk-frontend-4.1.0.min.css",
  "browser": "./dist/govuk-frontend-4.1.0.min.js",
  "default": "./dist/govuk-frontend-4.1.0.min.js"
}

Regarding "default" being a more universal implementation (with polyfills etc perhaps):

When using environment branches, always include a "default" condition where possible. Providing a "default" condition ensures that any unknown JS environments are able to use this universal implementation, which helps avoid these JS environments from having to pretend to be existing environments in order to support packages with conditional exports.

Longhand

But those "./govuk/" "./govuk-esm/" package entry points were for compatibility to let tools know "these paths are also available" without trying to resolve from "main" or "module" instead

"exports": {
  ".": {
    "import": "./govuk-esm/all.mjs",
    "require": "./govuk/all.js",
    "sass": "./govuk/all.scss",
    "style": "./dist/govuk-frontend-4.1.0.min.css",
    "browser": "./dist/govuk-frontend-4.1.0.min.js",
    "default": "./dist/govuk-frontend-4.1.0.min.js"
  },
  "./govuk/": "./govuk/",
  "./govuk-esm/": "./govuk-esm/"
}

But no, it doesn't open up that directory/path

You'll already be able to import a specific Sass stylesheet or nested polyfill directly

Typically you'd use them for backwards compatibility when files move:

"exports": {
  ".": "./dist/index.js",
  "./old-export": "./dist/legacy/new-export.js"
}

@vanitabarrett
Copy link
Contributor

vanitabarrett commented May 30, 2022

Thanks @colinrotherham that's really cleared a few things up! If you, or anyone else interested, would like to test with these changes, I've got a pre-release you can install by running:

npm install --save "alphagov/govuk-frontend#b516649e"

@colinrotherham
Copy link
Contributor Author

@vanitabarrett Perfect! Yep that does the job and the webpack guidance works now 👍

Beyond webpack can we leave this issue open until Mandatory file extensions are added?

One example is mocha (without Babel) running an ESM test file that imports govuk-frontend:

> mocha "./path/to/test.spec.mjs"

But without a bundler to help it's getting stuck on the first line of all.mjs:

import { nodeListForEach } from './common'

Without the .mjs extension it attempts ./common.js or ./common/index.js etc instead:

Error [ERR_MODULE_NOT_FOUND]: Cannot find module '/path/to/project/node_modules/govuk-frontend/govuk-esm/common' imported from /path/to/project/node_modules/govuk-frontend/govuk-esm/all.mjs
    at new NodeError (node:internal/errors:372:5)
    at finalizeResolution (node:internal/modules/esm/resolve:437:11)
    at moduleResolve (node:internal/modules/esm/resolve:1009:10)
    at defaultResolve (node:internal/modules/esm/resolve:1218:11)
    at ESMLoader.resolve (node:internal/modules/esm/loader:580:30)
    at ESMLoader.getModuleJob (node:internal/modules/esm/loader:294:18)
    at ModuleWrap.<anonymous> (node:internal/modules/esm/module_job:80:40)
    at link (node:internal/modules/esm/module_job:78:36)

Thanks again for the quick "exports" fix though 🎉

Glad it's helped others too

@lhokktyn
Copy link

@vanitabarrett that's fixed the issue for our use case too. Thanks for the quick response on this 👍

@vanitabarrett vanitabarrett moved this from Backlog 🗄 to In progress 📝 in Design System Sprint Board May 31, 2022
@vanitabarrett
Copy link
Contributor

vanitabarrett commented May 31, 2022

@colinrotherham Would you mind trying out this pre-release, when you get some time? It includes file extensions for all imported component JS files, so I'm hoping that may fix the issue 🤞

npm install --save "alphagov/govuk-frontend#872b5c1f"

Note: the implementation of this may change - we're currently weighing up shipping as .mjs or .js to make things simpler. But either way, this should help us understand if specifying all file extensions will fix the issue.

@colinrotherham
Copy link
Contributor Author

@vanitabarrett Sorry, few more in there!

(node:83409) Warning: To load an ES module, set "type": "module" in the package.json or use the .mjs extension.
(Use `node --trace-warnings ...` to show where the warning was created)

/path/to/project/node_modules/govuk-frontend/govuk-esm/vendor/polyfills/Function/prototype/bind.js:1
import '../../Object/defineProperty'
^^^^^^

SyntaxError: Cannot use import statement outside a module

See the .js extensions but without package.json { "type": "module" } here?

https://github.com/alphagov/govuk-frontend/blob/pre-release-full-resolve-js-path/govuk-esm/vendor/polyfills/Function/prototype/bind.js#L1

Can either change those imports to require() or rename to .mjs and add mandatory extensions there too?

@vanitabarrett
Copy link
Contributor

Ah interesting, thanks! Will take a look @colinrotherham

@vanitabarrett
Copy link
Contributor

@colinrotherham Have you got a basic setup you could share with me for testing this? When I tested it locally myself I didn't get that particular error 🤔

@colinrotherham
Copy link
Contributor Author

@vanitabarrett Here's a super minimal one:
https://github.com/colinrotherham/govuk-frontend-esm-sample

npm ci
npm start

Or via mocha

npm test

@vanitabarrett
Copy link
Contributor

Hi @colinrotherham , sorry for the slow response on this! I've been mulling over the best way for us to support this on GOV.UK Frontend (this PR if you're interested) . If you have the time to test another pre-release, you can installing it via:

npm install --save "alphagov/govuk-frontend#fcfe291f"

@colinrotherham
Copy link
Contributor Author

Hi @vanitabarrett great work 🎉

Yep it's working perfectly now with mocha (via ESM) and webpack without resolve: { fullySpecified: false }

@36degrees 36degrees added this to the v4.1.1 milestone Jun 15, 2022
@vanitabarrett vanitabarrett modified the milestones: v4.1.1, v4.2.0 Jun 20, 2022
This was referenced Jun 20, 2022
@vanitabarrett vanitabarrett moved this from Needs review 🔍 to Ready to release 🚀 in Design System Sprint Board Jun 20, 2022
@domoscargin domoscargin moved this from Ready to release 🚀 to Done 🏁 in Design System Sprint Board Jun 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛 bug Something isn't working the way it should (including incorrect wording in documentation) javascript
Projects
Development

Successfully merging a pull request may close this issue.

5 participants