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

ncu --doctor shows error with prepare script #1362

Closed
3 tasks done
adamhl8 opened this issue Jan 24, 2024 · 6 comments
Closed
3 tasks done

ncu --doctor shows error with prepare script #1362

adamhl8 opened this issue Jan 24, 2024 · 6 comments
Labels

Comments

@adamhl8
Copy link

adamhl8 commented Jan 24, 2024

  • I have searched for similar issues
  • I am using the latest version of npm-check-updates
  • I am using node >= 14.14

Related: #1170

Steps to Reproduce

  • Have a prepare script like: hustky install hooks
  • Run ncu -d -u

Current Behavior

For every dependency that ncu checks, I get an error about the prepare script.

yarn run prepare
Prepare script failed
  ✗ @emotion/styled ^11.10.6 → ^11.11.0

error Command failed with exit code 2.

Expected Behavior

I don't think ncu should run the prepare script by default, it should be opt-in (maybe with a --prepare flag or something). But as that would be potentially breaking for people, maybe just a flag to opt-out would be fine too.

@raineorshine
Copy link
Owner

Hi, thanks for reporting.

I'd like to better understand your use case, and then see what makes sense for npm-check-updates to support.

Why does husky install hooks fail? It seems like if a project has a prepare script, it should be expected to pass. npm prepare is automatically run after npm install, so a failed prepare script would presumably crash your normal install anyway.

https://stackoverflow.com/questions/44499912/why-is-npm-running-prepare-script-after-npm-install-and-how-can-i-stop-it

@adamhl8
Copy link
Author

adamhl8 commented Jan 24, 2024

Here's a repo that reproduces the issue: https://github.com/adamhl8/ncu-1362

I'm running ncu -d -u --doctorTest 'yarn test'

The prepare script works fine normally. It only fails when executed under ncu. I haven't been able to figure out why.

@raineorshine
Copy link
Owner

raineorshine commented Jan 25, 2024

Thanks. On my machine (OSX), the test script in the repo you provided is failing rather than the prepare script. It fails on the sed command (even without running ncu).

So I'm not sure if that properly reproduces your original issue.

raine[ncu-1362]% npm test

> [email protected] test
> vitest run && sed -i 's|expect(true)|expect(true).toBeFalsy()|' ./src/__tests__/index.test.tsx


 RUN  v0.29.8 /Users/raine/projects/ncu-1362

 ✓ src/__tests__/index.test.tsx (1)

 Test Files  1 passed (1)
      Tests  1 passed (1)
   Start at  19:33:46
   Duration  688ms (transform 45ms, setup 0ms, collect 23ms, tests 3ms)

sed: 1: "./src/__tests__/index.t ...": invalid command code .
raine[ncu-1362]% echo $?
1

@adamhl8
Copy link
Author

adamhl8 commented Jan 25, 2024

Sorry, I should have specified. That sed command has nothing to do with the issue itself and is not in the actual project I'm having this issue with. It's there because I need the test to pass initially so ncu --doctor continues and tries to update the dependencies. That sed command makes it so after that initial run the test fails so ncu will try to upgrade each dependency individually.

I'm on macOS as well. Are you using gnu-sed? The default, older version that ships with macOS might not support that syntax or something.

Edit: I changed the sed command in the repo to work with the default macOS sed.

@raineorshine
Copy link
Owner

Thanks! I was able to successfully reproduce it.

Internally, the options --depth=0 and --json are added to package manager commands such as npm list to ensure that the output can be parsed correctly. They are added to all package manager commands, which is usually harmless, however it turns out that yarn test --depth=0 --json behaves differently than npm test --depth=0 --json. yarn passes the option through to the test script, which causes sed to choke on --depth=0. Changing the order to yarn --depth=0 --json test fixes the issue.

I have published a fix to v16.14.13. Let me know if that helps.

@adamhl8
Copy link
Author

adamhl8 commented Jan 25, 2024

That fixed it. Thank you!

@adamhl8 adamhl8 closed this as completed Jan 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants