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

import/order order has different behavior across node versions - vscode forces xo to run in node v14 #103

Closed
novemberborn opened this issue Oct 23, 2021 · 10 comments

Comments

@novemberborn
Copy link

https://github.com/avajs/ava customizes the import ordering so that it deviates from XO 0.44. When I format AVA code using this extension, the imports are reordered according to the XO rules.

I haven't noticed any other rules being disregarded like this, but that may be due to the files I've been editing.

@spence-s
Copy link
Collaborator

spence-s commented Oct 23, 2021

This extensions does not do anything but apply the formatting directly from running the unaltered text of the file through xo. There are many other plugins and settings in VSCode that can reorder imports.

Can you verify that running xo path/to/test.js --fix has a different output than the extension and maybe expand a bit on why you think the extension is the issue?

@spence-s
Copy link
Collaborator

@novemberborn I tested on ava - and xo from cli is giving me same results as extension, and it looks to me like it's respecting the rules set up in the .xo-config.json.

@novemberborn
Copy link
Author

All I know is that it's a problem when I enable the extension.

It doesn't make sense why this would be the only rule that's disregarded though.

Perhaps there's something else VSCode does that is applied afterwards? However I don't run into this for projects that use the ESLint extension.

@spence-s
Copy link
Collaborator

spence-s commented Oct 23, 2021

@novemberborn - I just cloned ava and installed deps - then ran node_modules/.bin/xo from the command line and get about 120 import/order errors. I can go through each file reported from the command line and see the same report from the extension in vscode. I then look at your configuration - and the very top rule says to error for import/order and it looks like exactly what xo is doing from both the command line and from the extension.

Can you tell me specifically what file is looking incorrect to you?

@spence-s
Copy link
Collaborator

Maybe I am confused - maybe you are reporting on the problem that the lint errors are correct, but formatting still leaves another error - and the file doesn't get fully fixed?

If you format 2x it will fix the issues completely. This is a bug with the extension, can you confirm that this is the bug that you are experiencing?

@novemberborn
Copy link
Author

npx xo lib/cli.js reports no errors, npx xo --fix lib/cli.js makes no changes. When I open that same file in VSCode the Problems tab reports XO(import/order) errors on these three lines: https://github.com/avajs/ava/blob/a5ac79eb4a69066f166956da3277227bc241b54d/lib/cli.js#L2-L4

I then save (repeatedly) and the imports reorder to be completely alphabetical. Of course at this point npx xo lib/cli.js starts failing:

  lib/cli.js:5:1
  ✖  5:1  There should be at least one empty line between import groups  import/order
  ✖  6:1  node:fs import should occur before import of arrify            import/order
  ✖  7:1  node:path import should occur before import of arrify          import/order
  ✖  8:1  There should be at least one empty line between import groups  import/order
  ✖  8:1  node:process import should occur before import of arrify       import/order

My workspace configuration:

{
	"editor.formatOnSave": true,
	"eslint.enable": false,
	"xo.format.enable": true,
	"xo.enable": true,
	"[javascript]": {
		"editor.defaultFormatter": "samverschueren.linter-xo",
	},
	"editor.codeActionsOnSave": {
		"source.organizeImports": false
	},
	"prettier.enable": false
}

@spence-s
Copy link
Collaborator

spence-s commented Oct 25, 2021

Thanks so much for the detailed steps. This one was really bugging me as I could NOT reproduce your steps. The extension and the cli were 100% consistent for me.

However, I figured out the problem.

The problem is that eslint-plugin-import has some weird behavior that is not consistent across all versions of node. For node v14, I don't believe it properly supports the "node protocol" import syntax. However, is does properly support it if you are using node v16+.

The reason it is showing up in vscode and not in the cli for you - I would bet because you are using node v16 for cli but vscode is internally using its own bundled version nodejs which is node v14 (which is what I was using for the cli, and why I didn't see the problem initially).

Since eslint-plugin-import does not support the node import protocol for node 14 - it causes weird behavior within vscode only. If you downgrade your system node version to 14, and then run the cli again, you will see the same results that you see in the editor.

for reference see: import-js/eslint-plugin-import#2035

since eslint-plugin-import has questionable support for the node protocol - your best bet is to disable the rule:
"unicorn/prefer-node-protocol": "off"

Whenever I do this, everything works as expected.

The eslint extension jumps through more hoops than this extension does to resolve the system versions of node and allows users to give their own path to the node runtime. Because of this bug - I will leave this open and implement the same logic for this extension. However, it may some time until I get to it. For now the best course of action unfortunately is to disable this rule, I will add a note for this behavior on the readme.

TL;DR
My guess is that you are using node version 16+ while vscode internally uses its own node v14 which causes different behavior for eslint-plugin-import based on the "node protocol". Disable the node protocol rule like so: "unicorn/prefer-node-protocol": "off" for best consistency until I can get it fixed in the extension.

@spence-s spence-s changed the title import/order from .xo-config.json is disregarded import/order order has different behavior across node versions - vscode forces xo to run in node v14 Oct 25, 2021
@spence-s
Copy link
Collaborator

Actually - disabling that rule will still require you to rewrite all your imports manually to fix the vscode behavior... I am going to add the runtime option in now. Hold off on doing a rewrite - I will reply here with the correct solution soon so that you will get consisten results.

@spence-s
Copy link
Collaborator

spence-s commented Oct 25, 2021

@novemberborn

This option is in now.

  1. Update linter-xo in vscode to 3.6.

  2. Go to command line and run which node (or whatever method you have to get absolute path to node binary you want to use)

  3. go to your vscode settings, probably user settings so this will apply to all workspaces, and add
    "xo.runtime": "/path/to/node

  4. Restart VSCode for this to take effect.

  5. Open the XO output channel and ensure that the top line says "XO Server Starting in Node v{same version from cli}"

--
I just tested this on ava and it seems to work great for me now. Thanks for your patience with this!

@novemberborn
Copy link
Author

Wow, thanks for digging into this @spence-s! Kinda weird how that lint rule is Node.js version dependent.

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

No branches or pull requests

2 participants