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

feat: Support node16 for eslint 8 #279

Closed
wants to merge 1 commit into from

Conversation

scagood
Copy link

@scagood scagood commented May 14, 2024

seeing if node 16 is plausible for #210

@scagood
Copy link
Author

scagood commented May 14, 2024

I did also try node 14, but many child packages are not happy with that 👀

📁 Node 14 patch
diff --git a/lib/rules/no-unsupported-features/es-syntax.js b/lib/rules/no-unsupported-features/es-syntax.js
index d049027..3c0b434 100644
--- a/lib/rules/no-unsupported-features/es-syntax.js
+++ b/lib/rules/no-unsupported-features/es-syntax.js
@@ -47,7 +47,7 @@ const ruleMap = Object.entries(features).map(([ruleId, meta]) => {
     const ruleIdNegated = ruleId.replace(/^no-/, "")
     const ruleIdCamel = ruleIdNegated.replace(/-(\w)/g, firstMatchToUpper)
 
-    meta.aliases ??= []
+    meta.aliases = meta.aliases ?? []
     const ignoreNames = [ruleId, ruleIdNegated, ruleIdCamel, ...meta.aliases]
 
     for (const ignoreName of ignoreNames) {
diff --git a/lib/util/check-existence.js b/lib/util/check-existence.js
index fdfd897..4da6c3b 100644
--- a/lib/util/check-existence.js
+++ b/lib/util/check-existence.js
@@ -14,7 +14,7 @@ const getAllowModules = require("./get-allow-modules")
  */
 function markMissing(context, target) {
     // This should never happen... this is just a fallback for typescript
-    target.resolveError ??= `"${target.name}" is not found`
+    target.resolveError = target.resolveError ?? `"${target.name}" is not found`
 
     context.report({
         node: target.node,
diff --git a/lib/util/get-typescript-extension-map.js b/lib/util/get-typescript-extension-map.js
index b9e8ef1..8ec302b 100644
--- a/lib/util/get-typescript-extension-map.js
+++ b/lib/util/get-typescript-extension-map.js
@@ -48,7 +48,7 @@ function normalise(typescriptExtensionMap) {
         if (!typescript) {
             continue
         }
-        backward[javascript] ??= []
+        backward[javascript] = backward[javascript] ?? []
         backward[javascript].push(typescript)
     }
 
diff --git a/package.json b/package.json
index 69c1b22..088aef2 100644
--- a/package.json
+++ b/package.json
@@ -3,7 +3,7 @@
     "version": "17.6.0",
     "description": "Additional ESLint's rules for Node.js",
     "engines": {
-        "node": ">=16.0.0"
+        "node": "^14.17.0 || >=16.0.0"
     },
     "main": "lib/index.js",
     "types": "types/index.d.ts",
diff --git a/tests/lib/rules/shebang.js b/tests/lib/rules/shebang.js
index 7e09660..bfba249 100644
--- a/tests/lib/rules/shebang.js
+++ b/tests/lib/rules/shebang.js
@@ -1,5 +1,5 @@
 "use strict"
-const assert = require("assert/strict")
+const assert = require("assert")
 
 it("should export shebang as alias ", () => {
     const shebang = require("../../../lib/rules/shebang.js")
📁 Test errors
[~/github/opensource/eslint-plugin-n] (node-16*) $ volta run --node 14.17.0 npm run test                                                                                                                                                                               (docker-desktop|default)
npm WARN using --force I sure hope you know what you are doing.

> eslint-plugin-n@17.6.0 test /Users/scagood/github/opensource/eslint-plugin-n
> run-p lint:* test:types test:tests

npmnpm WARN using --force I sure hope you know what you are doing.
 WARN using --force I sure hope you know what you are doing.
npm WARN using --force I sure hope you know what you are doing.
npm WARN using --force I sure hope you know what you are doing.
npm WARN using --force I sure hope you know what you are doing.

> eslint-plugin-n@17.6.0 lint:eslint-docs /Users/scagood/github/opensource/eslint-plugin-n
> npm run update:eslint-docs -- --check


> eslint-plugin-n@17.6.0 lint:docs /Users/scagood/github/opensource/eslint-plugin-n
> markdownlint "**/*.md"


> eslint-plugin-n@17.6.0 lint:js /Users/scagood/github/opensource/eslint-plugin-n
> eslint .


> eslint-plugin-n@17.6.0 test:types /Users/scagood/github/opensource/eslint-plugin-n
> tsc --noEmit --emitDeclarationOnly false


> eslint-plugin-n@17.6.0 test:tests /Users/scagood/github/opensource/eslint-plugin-n
> npm run test:mocha "tests/lib/**/*.js"

/Users/scagood/github/opensource/eslint-plugin-n/node_modules/markdownlint-cli/markdownlint.js:279
  files ||= [];
        ^^^

SyntaxError: Unexpected token '||='
    at wrapSafe (internal/modules/cjs/loader.js:984:16)
    at Module._compile (internal/modules/cjs/loader.js:1032:27)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1097:10)
    at Module.load (internal/modules/cjs/loader.js:933:32)
    at Function.Module._load (internal/modules/cjs/loader.js:774:14)
    at Function.executeUserEntryPoint [as runMain] (internal/modules/run_main.js:72:12)
    at internal/main/run_main_module.js:17:47
npm WARN using --force I sure hope you know what you are doing.
npm WARN using --force I sure hope you know what you are doing.

> eslint-plugin-n@17.6.0 update:eslint-docs /Users/scagood/github/opensource/eslint-plugin-n
> eslint-doc-generator "--check"


> eslint-plugin-n@17.6.0 test:mocha /Users/scagood/github/opensource/eslint-plugin-n
> _mocha  --reporter progress --timeout 4000 "tests/lib/**/*.js"

[file-extension-in-import] Skip tests for 'import()'

/Users/scagood/github/opensource/eslint-plugin-n/node_modules/@typescript-eslint/parser/dist/parser.js:125
    services.emitDecoratorMetadata ??= options.emitDecoratorMetadata === true;
                                   ^^^

SyntaxError: Unexpected token '??='
    at wrapSafe (internal/modules/cjs/loader.js:984:16)
    at Module._compile (internal/modules/cjs/loader.js:1032:27)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1097:10)
    at Module.load (internal/modules/cjs/loader.js:933:32)
    at Function.Module._load (internal/modules/cjs/loader.js:774:14)
    at Module.require (internal/modules/cjs/loader.js:957:19)
    at require (internal/modules/cjs/helpers.js:88:18)
    at Object.<anonymous> (/Users/scagood/github/opensource/eslint-plugin-n/node_modules/@typescript-eslint/parser/dist/index.js:4:16)
    at Module._compile (internal/modules/cjs/loader.js:1068:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1097:10)
    at Module.load (internal/modules/cjs/loader.js:933:32)
    at Function.Module._load (internal/modules/cjs/loader.js:774:14)
    at Module.require (internal/modules/cjs/loader.js:957:19)
    at require (internal/modules/cjs/helpers.js:88:18)
    at Object.<anonymous> (/Users/scagood/github/opensource/eslint-plugin-n/tests/lib/rules/no-callback-literal.js:9:18)
    at Module._compile (internal/modules/cjs/loader.js:1068:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1097:10)
    at Module.load (internal/modules/cjs/loader.js:933:32)
    at Function.Module._load (internal/modules/cjs/loader.js:774:14)
    at ModuleWrap.<anonymous> (internal/modules/esm/translators.js:199:29)
    at ModuleJob.run (internal/modules/esm/module_job.js:152:23)
    at async Loader.import (internal/modules/esm/loader.js:177:24)
    at async formattedImport (/Users/scagood/github/opensource/eslint-plugin-n/node_modules/mocha/lib/nodejs/esm-utils.js:9:14)
    at async Object.exports.requireOrImport (/Users/scagood/github/opensource/eslint-plugin-n/node_modules/mocha/lib/nodejs/esm-utils.js:42:28)
    at async Object.exports.loadFilesAsync (/Users/scagood/github/opensource/eslint-plugin-n/node_modules/mocha/lib/nodejs/esm-utils.js:100:20)
    at async singleRun (/Users/scagood/github/opensource/eslint-plugin-n/node_modules/mocha/lib/cli/run-helpers.js:125:3)
    at async Object.exports.handler (/Users/scagood/github/opensource/eslint-plugin-n/node_modules/mocha/lib/cli/run.js:370:5)

Oops! Something went wrong! :(

ESLint: 8.57.0

TypeError: messageValue.replaceAll is not a function
Occurred while linting /Users/scagood/github/opensource/eslint-plugin-n/lib/rules/file-extension-in-import.js:93
Rule: "eslint-plugin/no-unused-placeholders"
    at CallExpression (/Users/scagood/github/opensource/eslint-plugin-n/node_modules/eslint-plugin-eslint-plugin/lib/rules/no-unused-placeholders.js:104:28)
    at ruleErrorHandler (/Users/scagood/github/opensource/eslint-plugin-n/node_modules/eslint/lib/linter/linter.js:1076:28)
    at /Users/scagood/github/opensource/eslint-plugin-n/node_modules/eslint/lib/linter/safe-emitter.js:45:58
    at Array.forEach (<anonymous>)
    at Object.emit (/Users/scagood/github/opensource/eslint-plugin-n/node_modules/eslint/lib/linter/safe-emitter.js:45:38)
    at NodeEventGenerator.applySelector (/Users/scagood/github/opensource/eslint-plugin-n/node_modules/eslint/lib/linter/node-event-generator.js:297:26)
    at NodeEventGenerator.applySelectors (/Users/scagood/github/opensource/eslint-plugin-n/node_modules/eslint/lib/linter/node-event-generator.js:326:22)
    at NodeEventGenerator.enterNode (/Users/scagood/github/opensource/eslint-plugin-n/node_modules/eslint/lib/linter/node-event-generator.js:340:14)
    at CodePathAnalyzer.enterNode (/Users/scagood/github/opensource/eslint-plugin-n/node_modules/eslint/lib/linter/code-path-analysis/code-path-analyzer.js:803:23)
    at /Users/scagood/github/opensource/eslint-plugin-n/node_modules/eslint/lib/linter/linter.js:1111:32

@scagood
Copy link
Author

scagood commented May 14, 2024

I am not sure we want to do this, but the experiment I think is worth while

@scagood scagood linked an issue May 14, 2024 that may be closed by this pull request
1 task
Comment on lines +13 to +15
function isBuiltin(name) {
return builtins.has(name)
}
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I originally tried using is-builtin-module@3.2.1 but it was missing node:test.

Thus I decided that, as we have the data already, we may as well use it :)

@scagood scagood marked this pull request as ready for review May 14, 2024 17:07
Copy link

@aladdin-add aladdin-add left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it just takes some time for users to upgrade their node.js. I don't see any reason we should support Node.js versions that the Node.js team don't even support.

@scagood
Copy link
Author

scagood commented May 16, 2024

it just takes some time for users to upgrade their node.js. I don't see any reason we should support Node.js versions that the Node.js team don't even support.

Yeah, I keep flipping back and forth on this one. I figured that this was okay because all of our dependencies were also fine with it 🤔

@aladdin-add
Copy link

I prefer no.

I figured that this was okay because all of our dependencies were also fine with it 🤔

It just happens to work, but there's no guarantee - they may introduce "breaking" changes in later releases.

@scagood
Copy link
Author

scagood commented May 16, 2024

Fair points, well made

@scagood scagood closed this May 16, 2024
@scagood scagood deleted the node-16 branch May 16, 2024 07:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

16.6.2: can n/no-missing-require allow resolvePaths ['./'] for relative imports?
2 participants