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

nodeVersion: false doesn't disable the related rules #598

Closed
XhmikosR opened this issue Aug 29, 2021 · 6 comments · Fixed by #761 or #762
Closed

nodeVersion: false doesn't disable the related rules #598

XhmikosR opened this issue Aug 29, 2021 · 6 comments · Fixed by #761 or #762

Comments

@XhmikosR
Copy link
Contributor

C:\Users\xmr\Desktop\foo>node -v && npm -v && ver
v14.17.5
6.14.14
Microsoft Windows [Version 10.0.19043.1165]

C:\Users\xmr\Desktop\foo>npm ls xo
[email protected] C:\Users\xmr\Desktop\foo
`-- [email protected]

I was trying to disable the following two rules:

  • "node/no-unsupported-features/es-builtins"
  • "node/no-unsupported-features/es-syntax"

but it doesn't have any effect. So then I saw that there's the nodeVersion option that could control this. I tried setting it to false but it doesn't work either.

A quick patch would be to change

...(enginesOptions && enginesOptions.node && semver.validRange(enginesOptions.node) ? {nodeVersion: enginesOptions.node} : {}),

-		...(enginesOptions && enginesOptions.node && semver.validRange(enginesOptions.node) ? {nodeVersion: enginesOptions.node} : {}),
+		...(xoOptions.nodeVersion !== false && enginesOptions && enginesOptions.node && semver.validRange(enginesOptions.node) ? {nodeVersion: enginesOptions.node} : {}),

But shouldn't it be possible to override the related rules?

@sindresorhus
Copy link
Member

A quick patch would be to change

Pull request welcome.

@XhmikosR
Copy link
Contributor Author

XhmikosR commented Sep 5, 2021

I can make a PR but I'm still not sure if it's the proper solution since IMHO it should be possible to override those rules only without overriding nodeVersion.

@sindresorhus
Copy link
Member

But shouldn't it be possible to override the related rules?

Ideally, yes, but it's complicated. Rule overrides can come from your package.json, but it can also come from a shareable config, and we override before all the config is resolved, so we cannot reliably check whether it's already set.

xo/lib/options-manager.js

Lines 309 to 313 in bc6e05d

if (options.nodeVersion) {
config.baseConfig.rules['node/no-unsupported-features/es-builtins'] = ['error', {version: options.nodeVersion}];
config.baseConfig.rules['node/no-unsupported-features/es-syntax'] = ['error', {version: options.nodeVersion, ignores: ['modules']}];
config.baseConfig.rules['node/no-unsupported-features/node-builtins'] = ['error', {version: options.nodeVersion}];
}

@fregante
Copy link
Member

fregante commented Jul 23, 2024

It looks like this was not fixed in #761, "nodeVersion": false, won't disable n rules

source/github-events/on-react-page-update.tsx:8:25
  ✖    8:25  The AbortSignal.any is still an experimental feature and is not supported until Node.js 20.3.0 (backported: ^18.17.0). The configured version range is >= 20.  n/no-unsupported-features/node-builtins

It did fix #613 though.

cc @melusc

@fregante
Copy link
Member

Pretty sure nodeVersion: false is being overridden here:

...(enginesOptions && enginesOptions.node && semver.validRange(enginesOptions.node) ? {nodeVersion: enginesOptions.node} : {}),

because xoOptions.nodeVersion is false but options.nodeVersion is still set to what's in the package.json after this line, so that's what's received by the rest of the code

@fregante
Copy link
Member

Confirmed to be fixed in v0.59.2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants