Skip to content
This repository has been archived by the owner on Aug 31, 2023. It is now read-only.

🐛 Enabling organizeImports and ignoring ["node_modules"] folder breaks rome #4434

Closed
1 task done
jrson83 opened this issue May 3, 2023 · 1 comment · Fixed by #4443
Closed
1 task done

🐛 Enabling organizeImports and ignoring ["node_modules"] folder breaks rome #4434

jrson83 opened this issue May 3, 2023 · 1 comment · Fixed by #4443
Assignees
Labels
A-Core Area: core A-LSP Area: language server protocol S-Bug: confirmed Status: report has been confirmed as a valid bug
Milestone

Comments

@jrson83
Copy link

jrson83 commented May 3, 2023

Environment information

CLI:
  Version:                      12.0.0
  Color support:                true

Platform:
  CPU Architecture:             x86_64
  OS:                           windows

Environment:
  ROME_LOG_DIR:                 unset
  NO_COLOR:                     unset
  TERM:                         "xterm-256color"
  JS_RUNTIME_VERSION:           "v18.14.0"
  JS_RUNTIME_NAME:              "node"
  NODE_PACKAGE_MANAGER:         "pnpm/8.2.0"

Rome Configuration:
  Status:                       Loaded successfully
  Formatter disabled:           false
  Linter disabled:              false
  Organize imports disabled:    false

Workspace:
  Open Documents:               0

What happened?

By default rome CLI ignores node_modules folders inside the workspace, but vscode extension does not. It makes no sense at all and just causes performance issues.

However, in order to fix this issue, we have to add node_modules folder to the linter ignore array.
When doing this and adding additionally organizeImports to rome.json, rome breaks in two ways.

  1. Enable organizeImports in rome.json by adding:
"organizeImports": {
  "enabled": true
}
  1. Add node_modules folder to ignore array:
"formatter": {
  // ..
  "ignore": ["node_modules"]
},
"linter": {
  // ..
  "ignore": ["node_modules"]
}

First issue

Clone my example

git clone https://github.com/jrson83/rome-issue.git

cd rome-issue

pnpm install
  • To reproduce open the node_modules/preact/package.json from the example

Opening any .json file from inside node_modules throws a notification Error in vscode:

Notification:
Request textDocument/codeAction failed.

Error output:

Connecting to "\\.\pipe\rome-service-12.0.0" ...
[Info  - 2:51:58 AM] Server initialized with PID: 22916
[Error - 2:54:13 AM] Request textDocument/codeAction failed.
  Message: The file node_modules\preact\package.json was ignored
  Code: -32603 
The file node_modules\preact\package.json was ignored

Second issue

git clone https://github.com/jrson83/rome-issue.git

cd rome-issue

pnpm install

Opening any .json file from inside node_modules and then opening any .json file from outside node_modules. Then ignoring the error above, restarting the IDE with both .json files still open, vscode will be unable to locate any schema from default schemas.

  • To reproduce open the node_modules/preact/package.json from the example, then open .vscode/settings.json and restart the IDE.
Unable to load schema from 'vscode://schemas/settings/folder': cannot open vscode://schemas/settings/folder. Detail: Unable to resolve text model content for resource vscode://schemas/settings/folder.(768)

2023-05-03_03h04_38

Expected result

Rome should ignore any **/node_modules/** folder inside a workspace by default.

Code of Conduct

  • I agree to follow Rome's Code of Conduct
@jrson83 jrson83 added the S-To triage Status: user report of a possible bug that needs to be triaged label May 3, 2023
@ematipico ematipico added S-Bug: confirmed Status: report has been confirmed as a valid bug A-Core Area: core A-LSP Area: language server protocol and removed S-To triage Status: user report of a possible bug that needs to be triaged labels May 4, 2023
@ematipico ematipico self-assigned this May 4, 2023
@ematipico ematipico added this to the v12.1.0 milestone May 4, 2023
@jrson83
Copy link
Author

jrson83 commented May 8, 2023

@ematipico thank you!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A-Core Area: core A-LSP Area: language server protocol S-Bug: confirmed Status: report has been confirmed as a valid bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants