Skip to content

Commit

Permalink
invalidate ESM imports that were valid in Node 10
Browse files Browse the repository at this point in the history
  • Loading branch information
giltayar committed Jan 28, 2022
1 parent 6a9adce commit b14f43f
Show file tree
Hide file tree
Showing 23 changed files with 117 additions and 168 deletions.
3 changes: 2 additions & 1 deletion .nycrc
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@
],
"exclude": [
"lib/rules/no-hide-core-modules.js",
"lib/rules/no-unsupported-features.js"
"lib/rules/no-unsupported-features.js",
"lib/converted-esm/*.js"
],
"reporter": [
"lcov",
Expand Down
23 changes: 0 additions & 23 deletions docs/rules/file-extension-in-import.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ This rule has a string option and an object option.
"error",
"always" or "never",
{
"tryExtensions": [".js", ".json", ".node"],
".xxx": "always" or "never",
}
]
Expand All @@ -36,7 +35,6 @@ This rule has a string option and an object option.

- `"always"` (default) requires file extensions in `import`/`export` declarations.
- `"never"` disallows file extensions in `import`/`export` declarations.
- `tryExtensions` is the file extensions to resolve import paths. Default is `[".js", ".json", ".node"]`.
- `.xxx` is the overriding setting for specific file extensions. You can use arbitrary property names which start with `.`.

#### always
Expand Down Expand Up @@ -90,27 +88,6 @@ import styles from "./styles.css"
import logo from "./logo.png"
```

### Shared Settings

The following options can be set by [shared settings](http://eslint.org/docs/user-guide/configuring.html#adding-shared-settings).
Several rules have the same option, but we can set this option at once.

- `tryExtensions`

```js
// .eslintrc.js
module.exports = {
"settings": {
"node": {
"tryExtensions": [".js", ".json", ".node"]
}
},
"rules": {
"n/file-extension-in-import": "error"
}
}
```

## 🔎 Implementation

- [Rule source](../../lib/rules/file-extension-in-import.js)
Expand Down
10 changes: 0 additions & 10 deletions docs/rules/no-extraneous-import.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ This rule warns `import` declarations of extraneous modules.
"n/no-extraneous-import": ["error", {
"allowModules": [],
"resolvePaths": [],
"tryExtensions": []
}]
}
}
Expand Down Expand Up @@ -48,21 +47,13 @@ If a path is relative, it will be resolved from CWD.

Default is `[]`

#### tryExtensions

When an import path does not exist, this rule checks whether or not any of `path.js`, `path.json`, and `path.node` exists.
`tryExtensions` option is the extension list this rule uses at the time.

Default is `[".js", ".json", ".node"]`.

### Shared Settings

The following options can be set by [shared settings](http://eslint.org/docs/user-guide/configuring.html#adding-shared-settings).
Several rules have the same option, but we can set this option at once.

- `allowModules`
- `resolvePaths`
- `tryExtensions`

```js
// .eslintrc.js
Expand All @@ -71,7 +62,6 @@ module.exports = {
"node": {
"allowModules": ["electron"],
"resolvePaths": [__dirname],
"tryExtensions": [".js", ".json", ".node"]
}
},
"rules": {
Expand Down
10 changes: 0 additions & 10 deletions docs/rules/no-missing-import.md
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ import existingModule from "existing-module";
"n/no-missing-import": ["error", {
"allowModules": [],
"resolvePaths": ["/path/to/a/modules/directory"],
"tryExtensions": [".js", ".json", ".node"]
}]
}
}
Expand Down Expand Up @@ -68,21 +67,13 @@ If a path is relative, it will be resolved from CWD.

Default is `[]`

#### tryExtensions

When an import path does not exist, this rule checks whether or not any of `path.js`, `path.json`, and `path.node` exists.
`tryExtensions` option is the extension list this rule uses at the time.

Default is `[".js", ".json", ".node"]`.

### Shared Settings

The following options can be set by [shared settings](http://eslint.org/docs/user-guide/configuring.html#adding-shared-settings).
Several rules have the same option, but we can set this option at once.

- `allowModules`
- `resolvePaths`
- `tryExtensions`

```js
// .eslintrc.js
Expand All @@ -91,7 +82,6 @@ module.exports = {
"node": {
"allowModules": ["electron"],
"resolvePaths": [__dirname],
"tryExtensions": [".js", ".json", ".node"]
}
},
"rules": {
Expand Down
11 changes: 0 additions & 11 deletions docs/rules/no-unpublished-import.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ Then this rule warns `import` declarations in \*published\* files if the `import
"n/no-unpublished-import": ["error", {
"allowModules": [],
"convertPath": null,
"tryExtensions": [".js", ".json", ".node"]
}]
}
}
Expand Down Expand Up @@ -64,7 +63,6 @@ For example:
"convertPath": {
"src/**/*.jsx": ["^src/(.+?)\\.jsx$", "lib/$1.js"]
},
"tryExtensions": [".js", ".jsx", ".json"]
}]
}
}
Expand Down Expand Up @@ -104,21 +102,13 @@ For example:
In this style, this option has the following shape as the same expression as above: `{include: [<targetFiles>], replace: [<fromRegExp>, <toString>]}`.
In addition, we can specify glob patterns to exclude files.

#### tryExtensions

When an import path does not exist, this rule checks whether or not any of `path.js`, `path.json`, and `path.node` exists.
`tryExtensions` option is the extension list this rule uses at the time.

Default is `[".js", ".json", ".node"]`.

### Shared Settings

The following options can be set by [shared settings](http://eslint.org/docs/user-guide/configuring.html#adding-shared-settings).
Several rules have the same option, but we can set this option at once.

- `allowModules`
- `convertPath`
- `tryExtensions`

For Example:

Expand All @@ -130,7 +120,6 @@ For Example:
"convertPath": {
"src/**/*.jsx": ["^src/(.+?)\\.jsx$", "lib/$1.js"]
},
"tryExtensions": [".js", ".jsx", ".json"]
}
},
"rules": {
Expand Down
8 changes: 1 addition & 7 deletions lib/rules/file-extension-in-import.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@

const path = require("path")
const fs = require("fs")
const getTryExtensions = require("../util/get-try-extensions")
const visitImport = require("../util/visit-import")
const packageNamePattern = /^(?:@[^/\\]+[/\\])?[^/\\]+$/u
const corePackageOverridePattern = /^(?:assert|async_hooks|buffer|child_process|cluster|console|constants|crypto|dgram|dns|domain|events|fs|http|http2|https|inspector|module|net|os|path|perf_hooks|process|punycode|querystring|readline|repl|stream|string_decoder|sys|timers|tls|trace_events|tty|url|util|v8|vm|worker_threads|zlib)[/\\]$/u
Expand Down Expand Up @@ -53,7 +52,6 @@ module.exports = {
{
type: "object",
properties: {
tryExtensions: getTryExtensions.schema,
},
additionalProperties: {
enum: ["always", "never"],
Expand Down Expand Up @@ -83,11 +81,7 @@ module.exports = {
const originalExt = path.extname(name)
const resolvedExt = path.extname(filePath)
const existingExts = getExistingExtensions(filePath)
if (!resolvedExt && existingExts.length !== 1) {
// Ignore if the file extension could not be determined one.
return
}
const ext = resolvedExt || existingExts[0]
const ext = resolvedExt || existingExts.join(' or ')
const style = overrideStyle[ext] || defaultStyle

// Verify.
Expand Down
4 changes: 1 addition & 3 deletions lib/rules/no-extraneous-import.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ const checkExtraneous = require("../util/check-extraneous")
const getAllowModules = require("../util/get-allow-modules")
const getConvertPath = require("../util/get-convert-path")
const getResolvePaths = require("../util/get-resolve-paths")
const getTryExtensions = require("../util/get-try-extensions")
const visitImport = require("../util/visit-import")

module.exports = {
Expand All @@ -29,8 +28,7 @@ module.exports = {
properties: {
allowModules: getAllowModules.schema,
convertPath: getConvertPath.schema,
resolvePaths: getResolvePaths.schema,
tryExtensions: getTryExtensions.schema,
resolvePaths: getResolvePaths.schema
},
additionalProperties: false,
},
Expand Down
2 changes: 0 additions & 2 deletions lib/rules/no-missing-import.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
const checkExistence = require("../util/check-existence")
const getAllowModules = require("../util/get-allow-modules")
const getResolvePaths = require("../util/get-resolve-paths")
const getTryExtensions = require("../util/get-try-extensions")
const visitImport = require("../util/visit-import")

module.exports = {
Expand All @@ -27,7 +26,6 @@ module.exports = {
type: "object",
properties: {
allowModules: getAllowModules.schema,
tryExtensions: getTryExtensions.schema,
resolvePaths: getResolvePaths.schema,
},
additionalProperties: false,
Expand Down
4 changes: 1 addition & 3 deletions lib/rules/no-unpublished-import.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ const checkPublish = require("../util/check-publish")
const getAllowModules = require("../util/get-allow-modules")
const getConvertPath = require("../util/get-convert-path")
const getResolvePaths = require("../util/get-resolve-paths")
const getTryExtensions = require("../util/get-try-extensions")
const visitImport = require("../util/visit-import")

module.exports = {
Expand All @@ -29,8 +28,7 @@ module.exports = {
properties: {
allowModules: getAllowModules.schema,
convertPath: getConvertPath.schema,
resolvePaths: getResolvePaths.schema,
tryExtensions: getTryExtensions.schema,
resolvePaths: getResolvePaths.schema
},
additionalProperties: false,
},
Expand Down
58 changes: 43 additions & 15 deletions lib/util/import-target.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,24 +15,51 @@ const {defaultResolve: importResolve} = require("../converted-esm/import-meta-re
* @param {string} id The id to resolve.
* @param {object} options The options of node-resolve module.
* It requires `options.basedir`.
* @param {'import' | 'require'} moduleType - whether the target was require-ed or imported
* @returns {string|null} The resolved path.
*/
function getFilePath(isModule, id, options) {
try {
return resolve.sync(id, options)
} catch (_err) {
try {
const {url} = importResolve(id, {
parentURL: pathToFileURL(path.join(options.basedir, 'dummy-file.js')).href,
function getFilePath(isModule, id, options, moduleType) {
if (moduleType === "import") {
const paths = options.paths && options.paths.length > 0 ?
options.paths.map(p => path.resolve(process.cwd(), p)) :
[options.basedir]
for (const aPath of paths) {
try {
const {url} = importResolve(id, {
parentURL: pathToFileURL(path.join(aPath, 'dummy-file.mjs')).href,
conditions: ['node', 'import', 'require']
})
})

if (url) {
return fileURLToPath(url)
}
} catch (e) {
continue
}
}

if (isModule) {
return null
}
return path.resolve((options.paths && options.paths[0]) || options.basedir, id)
}
else {
try {
return resolve.sync(id, options)
} catch (_err) {
try {
const {url} = importResolve(id, {
parentURL: pathToFileURL(path.join(options.basedir, 'dummy-file.js')).href,
conditions: ['node', 'require']
})

return fileURLToPath(url)
} catch (err) {
if (isModule) {
return null
return fileURLToPath(url)
} catch (err) {
if (isModule) {
return null
}
return path.resolve(options.basedir, id)
}
return path.resolve(options.basedir, id)
}
}
}
Expand Down Expand Up @@ -63,8 +90,9 @@ module.exports = class ImportTarget {
* @param {ASTNode} node - The node of a `require()` or a module declaraiton.
* @param {string} name - The name of an import target.
* @param {object} options - The options of `node-resolve` module.
* @param {'import' | 'require'} moduleType - whether the target was require-ed or imported
*/
constructor(node, name, options) {
constructor(node, name, options, moduleType) {
const isModule = !/^(?:[./\\]|\w+:)/u.test(name)

/**
Expand All @@ -84,7 +112,7 @@ module.exports = class ImportTarget {
* If the target is a module and it does not exist then this is `null`.
* @type {string|null}
*/
this.filePath = getFilePath(isModule, name, options)
this.filePath = getFilePath(isModule, name, options, moduleType)

/**
* The module name of this import target.
Expand Down
2 changes: 1 addition & 1 deletion lib/util/visit-import.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ module.exports = function visitImport(
const name = sourceNode && stripImportPathParams(sourceNode.value)
// Note: "999" arbitrary to check current/future Node.js version
if (name && (includeCore || !isCoreModule(name, "999"))) {
targets.push(new ImportTarget(sourceNode, name, options))
targets.push(new ImportTarget(sourceNode, name, options, 'import'))
}
},

Expand Down
2 changes: 1 addition & 1 deletion lib/util/visit-require.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ module.exports = function visitRequire(
const name = rawName && stripImportPathParams(rawName)
// Note: "999" arbitrary to check current/future Node.js version
if (name && (includeCore || !isCoreModule(name, "999"))) {
targets.push(new ImportTarget(targetNode, name, options))
targets.push(new ImportTarget(targetNode, name, options, 'require'))
}
}

Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit b14f43f

Please sign in to comment.