Fix nodeVersion:false not working

[XhmikosR]

Fixes #598

PS. I'm not familiar with the codebase and its tests, but if someone guides me I could try adding a test. Alternatively, feel free to push to the branch :)

[sindresorhus]

You could probably just update the test here:

xo/test/options-manager.js

Lines 233 to 244 in a2eada4

	test('buildConfig: nodeVersion: false', t => {
	const config = manager.buildConfig({nodeVersion: false});
	// Override all the rules specific to Node.js version
	t.is(config.baseConfig.rules['prefer-object-spread'], 'off');
	t.is(config.baseConfig.rules['prefer-rest-params'], 'off');
	t.is(config.baseConfig.rules['prefer-destructuring'], 'off');
	t.is(config.baseConfig.rules['promise/prefer-await-to-then'], 'off');
	t.is(config.baseConfig.rules['unicorn/prefer-flat-map'], 'off');
	t.is(config.baseConfig.rules['node/prefer-promises/dns'], 'off');
	t.is(config.baseConfig.rules['node/prefer-promises/fs'], 'off');
	});

[XhmikosR]

I added 3 tests like in buildConfig: engines: undefined but these were passing before anyway, so I'm not sure if they are right...

[XhmikosR]

Unrelated to this but I noticed it while running the tests: there's something wrong with ava.

My VM has 4 threads yet there are spawned too many processes and makes the system unresponsive...

[sindresorhus]

Did you manually verify that this PR fixes the issue for you?

[XhmikosR]

When I tried it yeah, but now I see it's wrong. I'll mark it as a draft and have another try at it later.

What worked for me is this patch but then the current tests fail so this can't be right either:

 lib/options-manager.js | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/lib/options-manager.js b/lib/options-manager.js
index e2dd2d4..b83a453 100644
--- a/lib/options-manager.js
+++ b/lib/options-manager.js
@@ -207,6 +207,8 @@ const mergeOptions = (options, xoOptions = {}, enginesOptions = {}) => {
 		...options,
 	});
 
+	mergedOptions.nodeVersion = xoOptions.nodeVersion === false ? false : xoOptions.nodeVersion;
+
 	mergedOptions.extensions = [...DEFAULT_EXTENSION, ...(mergedOptions.extensions || [])];
 	mergedOptions.ignores = getIgnores(mergedOptions);
 

[XhmikosR]

This is what works for me, but tests break miserably. I'm going to close this for someone else more familiar with the codebase to handle.

[devinrhode2]

IMO we should keep this issue open, for visibility