fix: add missing file extension in imports of the npm package#301
Merged
Conversation
Webpack follows node.js rules for imports: > In .mjs or .js files with a package.json whose ‘type’ is ‘module’, imports must be fully specified. This means that the extension must be included. This is how the ESM is defined in node.js The ‘type’ of the library is defined as ‘module’ in the package.json file, so this behaviour is applied. So, the imports have been updated to include the .js file extension. An eslint plugin is used to enforce and automatically fix missing file extension in imports. Previously, as the imports didn't include the file extension in version 0.7.0 of the library, webpack raised errors like in the following: ```ERROR in ./node_modules/@process-analytics/bv-experimental-add-ons/dist/index.js 16:0-32 Module not found: Error: Can't resolve './bpmn-elements' in '/user/app/node_modules/@process-analytics/bv-experimental-add-ons/dist' Did you mean 'bpmn-elements.js'? BREAKING CHANGE: The request './bpmn-elements' 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. ERROR in ./node_modules/@process-analytics/bv-experimental-add-ons/dist/index.js 17:0-24 Module not found: Error: Can't resolve './paths' in '/user/app/node_modules/@process-analytics/bv-experimental-add-ons/dist' Did you mean 'paths.js'? BREAKING CHANGE: The request './paths' 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. ERROR in ./node_modules/@process-analytics/bv-experimental-add-ons/dist/index.js 18:0-26 Module not found: Error: Can't resolve './plugins' in '/user/app/node_modules/@process-analytics/bv-experimental-add-ons/dist' Did you mean 'index.js'? BREAKING CHANGE: The request './plugins' 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. ERROR in ./node_modules/@process-analytics/bv-experimental-add-ons/dist/index.js 19:0-34 Module not found: Error: Can't resolve './plugins-support' in '/user/app/node_modules/@process-analytics/bv-experimental-add-ons/dist' Did you mean 'plugins-support.js'? BREAKING CHANGE: The request './plugins-support' failed to resolve only because it was resolved as fully specified ... ```
|
🎊 PR Preview 151f0af has been successfully built and deployed to https://process-analytics-bv-experimental-add-ons-demo-pr-301.surge.sh 🕐 Build time: 0.01s
🤖 By surge-preview |
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Webpack follows node.js rules for imports:
The ‘type’ of the library is defined as ‘module’ in the package.json file, so this behaviour is applied. So, the imports have been updated to include the .js file extension. An eslint plugin is used to enforce and automatically fix missing file extension in imports.
Previously, as the imports didn't include the file extension in version 0.7.0 of the library, webpack raised errors like in the following:
Notes
webpack details
Extensions in import and TypeScript transpilation
TypeScript has not planned to change the imports declared in the ts files at transpilation time. See microsoft/TypeScript#16577 (comment)
New eslint rule to enforce js extension in imports
See https://github.com/eslint-community/eslint-plugin-n/blob/85b794508a0fb92e021c09e63378314093772640/docs/rules/file-extension-in-import.md
https://github.com/import-js/eslint-plugin-import/blob/v2.23.4/docs/rules/extensions.md was also experimented (see below for more details).
About this problem in
bpmn-visualizationThere is no problem in
bpmn-visualization. We provide a single bundle file which is referenced in the package.json. So, there is no JS files importing other modules.Investigating using the
extensionsrule ineslint-plugin-import.I have not been able to get it to work. Also,
eslint-plugin-importdoes not provide automatic correction for erroneous imports, whereaseslint-plugin-ndoes. It's a pain. So, for all these reasons, I've decided to stick witheslint-plugin-n, at least for now.I have followed
I got errors like these
The problem may be due to an unresolved issue: import-js/eslint-plugin-import#2111
Tasks
No need to add a new dependency withI have not been able to make it work (see above), so keep usingeslint-plugin-n. Use the rules available in the import plugin https://github.com/import-js/eslint-plugin-import/blob/v2.23.4/docs/rules/extensions.md.eslint-plugin-n.a TypeScript configuration may also enforce this. See A compiler option to enforce file extensions. microsoft/TypeScript#42813 and moduleResolution bundler https://www.typescriptlang.org/docs/handbook/modules/guides/choosing-compiler-options.html#considerations-for-bundling-libraries--> not accurate for our use case