antfu-collective / ni

💡 Use the right package manager
MIT License
6.42k stars 210 forks source link

When `nr` is run concurrently, `nr` may not work #144

Closed neoki07 closed 1 year ago

neoki07 commented 1 year ago

Describe the bug

When I replaced pnpm run with nr in my project, the nr command suddenly stopped working.

I investigated the issue and found that the _storage.json, which stores the last run command, was not in the correct JSON format.

ni-storage-bug

The nr command first parses the contents of this JSON file, and I suspect that a syntax error is causing it to exit abnormally.

Cause of Issue

I'm not entirely certain, but I suspect this issue is caused by simultaneous writes to _storage.json when the nr command is run concurrently (my project uses lint-staged, which is where the concurrency is happening).

I found a similar issue in the node repository, which may have the same cause: nodejs/node#26338

Proposed Solutions

I have two potential solutions:

  1. Catch SyntaxError during JSON.parse(<_storage.json>)
    • The last run command cannot be used when the issue occurs. However, this solution is easy to implement.
    • If _storage.json is corrupted for any other reason, it will work.
  2. Implement locking when writing to _storage.json

If changes are required for this issue, I would like to work on a pull request. However, I have no experience working on pull requests, so it may take some effort on my part...

Related Issues

This issue may not be related to concurrent running, but this is related to the corruption of _storage.json: #141

Reproduction

https://github.com/ot07/nr-command-concurrent-running-storage-file-issue

System Info

System:
  OS: macOS 13.1
  CPU: (16) x64 Intel(R) Core(TM) i9-9880H CPU @ 2.30GHz
  Memory: 38.07 MB / 32.00 GB
  Shell: 5.8.1 - /bin/zsh
Binaries:
  Node: 18.15.0 - ~/.asdf/installs/nodejs/18.15.0/bin/node
  Yarn: 1.22.19 - ~/.asdf/shims/yarn
  npm: 9.5.0 - ~/.asdf/plugins/nodejs/shims/npm
Browsers:
  Brave Browser: 110.1.48.164
  Chrome: 111.0.5563.64
  Safari: 16.2

Used Package Manager

npm

Validations

neoki07 commented 1 year ago

I just fixed the reproduction.