rome / tools

Unified developer tools for JavaScript, TypeScript, and the web
https://docs.rome.tools/
MIT License
23.8k stars 665 forks source link

πŸ› Rome with pnpm workspace fails #3621

Closed yagiz-noonlight closed 1 year ago

yagiz-noonlight commented 1 year ago

Environment information

➜  ~ npx rome rage
CLI:
  Version:              10.0.0
  Color support:        true

Platform:
  CPU Architecture:     aarch64
  OS:                   macos

Environment:
  ROME_LOG_DIR:         unset
  NO_COLOR:             unset
  TERM:                 "xterm-256color"

Rome Configuration:
  Status:               unset

Workspace:
  Open Documents:       0

Discovering running Rome servers...

Running Rome Server: ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

β„Ή The client isn't connected to any server but rage discovered this running Rome server.

βœ– Workspace rage failed: Method not found

Rome Server Log:

⚠ Please review the content of the log file before sharing it publicly as it may contain sensitive information:
  * Path names that may reveal your name, a project name, or the name of your employer.
  * Source code

β”œβ”€369287484ms INFO rome_lsp::server Starting Rome Language Server...
β”œβ”€369307284ms INFO rome_lsp::server Starting Rome Language Server...
β”œβ”€369307285ms ERROR tower_lsp::transport failed to encode message: failed to encode response: Socket is not connected (os error 57)


### What happened?

```➜  developer npx rome check .
/Users/yagiz_nizipli/Developer/developer/node_modules/.pnpm/rome@10.0.0/node_modules/rome/bin/rome:28
        throw result.error;
        ^

Error: spawnSync /Users/yagiz_nizipli/Developer/developer/node_modules/.pnpm/@rometools+cli-darwin-arm64@10.0.0/node_modules/@rometools/cli-darwin-arm64/rome EACCES
    at Object.spawnSync (node:internal/child_process:1110:20)
    at Object.spawnSync (node:child_process:857:24)
    at Object.<anonymous> (/Users/yagiz_nizipli/Developer/developer/node_modules/.pnpm/rome@10.0.0/node_modules/rome/bin/rome:21:42)
    at Module._compile (node:internal/modules/cjs/loader:1159:14)
    at Module._extensions..js (node:internal/modules/cjs/loader:1213:10)
    at Module.load (node:internal/modules/cjs/loader:1037:32)
    at Module._load (node:internal/modules/cjs/loader:878:12)
    at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:81:12)
    at node:internal/main/run_main_module:23:47 {
  errno: -13,
  code: 'EACCES',
  syscall: 'spawnSync /Users/yagiz_nizipli/Developer/developer/node_modules/.pnpm/@rometools+cli-darwin-arm64@10.0.0/node_modules/@rometools/cli-darwin-arm64/rome',
  path: '/Users/yagiz_nizipli/Developer/developer/node_modules/.pnpm/@rometools+cli-darwin-arm64@10.0.0/node_modules/@rometools/cli-darwin-arm64/rome',
  spawnargs: [ 'check', '.' ]
}```

### Expected result

To run correctly.

### Code of Conduct

- [X] I agree to follow Rome's Code of Conduct
MichaReiser commented 1 year ago

I understand that you installed rome with pnpm. Is this correct?

If so, can you try running rome with pnpm exec rome

nicksrandall commented 1 year ago

I am also seeing this issue. pnpm exec rome spits out the same error as above.

MichaReiser commented 1 year ago

I haven't used pnpm workspaces myself. Does either of you have a project that I could use (or PR in that project) to reproduce the issue?

yagiz-noonlight commented 1 year ago

Unfortunately not. These are steps:

  1. mkdir test-folder && cd test-folder
  2. npm init -y
  3. touch pnpm-workspace.yml && nano pnpm-workspace.yml
  4. Add packages: \n - 'packages/*'
  5. run pnpm add -D rome -w in root directory
  6. add rome format . to any scripts.format in package.json
  7. run pnpm run format
leops commented 1 year ago

Since the spawnSync function fails with the code EACCES is suspect this is caused by incorrect permissions being set on the rome prebuilt binary. The rome package makes use of a postinstall script (here: https://github.com/rome/tools/blob/1c08b8f6f31b803111752fa8169c740ae82daeab/npm/rome/scripts/postinstall.js) to ensure the permissions are set correctly on the binary file, maybe the script isn't being run correctly ?

MichaReiser commented 1 year ago

Unfortunately not. These are steps:

1. mkdir test-folder && cd test-folder

2. npm init -y

3. touch pnpm-workspace.yml && nano pnpm-workspace.yml

4. Add `packages: \n - 'packages/*'`

5. run `pnpm add -D rome -w` in root directory

6. add `rome format .` to any `scripts.format` in package.json

7. run `pnpm run format`

I followed your instruction and added a format command to one of the packages and to the workspace root and both run successfully. Does the setup look correct? I haven't yet tested it on MacOs

pnpm-workspace-test.zip

Do you see the message that the post install scripts were running successfully when running pnpm i?

Packages are copied from the content-addressable store to the virtual store.
  Content-addressable store is at: /Users/micha/Library/pnpm/store/v3
  Virtual store is at:             node_modules/.pnpm
node_modules/.pnpm/rome@10.0.0/node_modules/rome: Running postinstall script, done in 181ms <---- THIS
Progress: resolved 2, reused 2, downloaded 0, added 2, done
ydcjeff commented 1 year ago

I think this isn't specially tied to pnpm workspace, but only with pnpm. Rome failed to run with pnpm (edited: macOS).

Reproduction:

  1. pnpm create vite test-vite
  2. Choose vanilla JS
  3. pnpm i, pnpm add rome -D
  4. pnpm exec rome format --write .
  5. You will see the errors below.

Screenshot 2022-11-10 at 19 05 12

Workaround:

Run node node_modules/.pnpm/rome@10.0.0/node_modules/rome/scripts/postinstall.js and pnpm exec rome format works. Looks like postinstall script doesn't run.

MichaReiser commented 1 year ago

Thanks for the vite example. It allows me to reproduce the issue on macos (linux is fine)

lzm0x219 commented 1 year ago

mark

ematipico commented 1 year ago

Thanks for the vite example. It allows me to reproduce the issue on macos (linux is fine)

I can't replicate the issue on macOS using the instructions provided. The posinstall script works:

Packages are copied from the content-addressable store to the virtual store.
  Content-addressable store is at: /Users/*/Library/pnpm/store/v3
  Virtual store is at:             node_modules/.pnpm
node_modules/.pnpm/esbuild@0.15.13/node_modules/esbuild: Running postinstall script, done in 423ms
Progress: resolved 43, reused 11, downloaded 6, added 17, done
node_modules/.pnpm/rome@10.0.1/node_modules/rome: Running postinstall script, done in 160ms
lzm0x219 commented 1 year ago

Thanks for the vite example. It allows me to reproduce the issue on macos (linux is fine)

I can't replicate the issue on macOS using the instructions provided. The posinstall script works:

Packages are copied from the content-addressable store to the virtual store.
  Content-addressable store is at: /Users/*/Library/pnpm/store/v3
  Virtual store is at:             node_modules/.pnpm
node_modules/.pnpm/esbuild@0.15.13/node_modules/esbuild: Running postinstall script, done in 423ms
Progress: resolved 43, reused 11, downloaded 6, added 17, done
node_modules/.pnpm/rome@10.0.1/node_modules/rome: Running postinstall script, done in 160ms

You can use this template, you can reproduce the problem.

image
ematipico commented 1 year ago

Great, I can re create the issue here. For some unknown reason the postinstall script is not run in this using this template.

lzm0x219 commented 1 year ago

Great, I can re create the issue here. For some unknown reason the postinstall script is not run in this using this template.

Yes, thank you very much for coming to see the reason for this issue.πŸ₯³

lzm0x219 commented 1 year ago

This question is related to pnpm. https://github.com/pnpm/pnpm/issues/4649

Create a .npmrc file in the current project:

side-effects-cache = false

remove node_modules and reinstall.

richerfu commented 1 year ago

This question is related to pnpm. pnpm/pnpm#4649

Create a .npmrc file in the current project:

side-effects-cache = false

remove node_modules and reinstall.

same problem, this worked

MichaReiser commented 1 year ago

We changed the way we set the executable bits. Can you try out the latest nightly release and test if the problem remains?

pnpm add rome@nightly
lzm0x219 commented 1 year ago

We changed the way we set the executable bits. Can you try out the latest nightly release and test if the problem remains?

pnpm add rome@nightly

it's work.