- 
                Notifications
    You must be signed in to change notification settings 
- Fork 6.3k
fix(npm): use correct flags and update version for npm #5533
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
Conversation
         jsjoeio
  
      
      
      commented
      
            jsjoeio
  
      
      
      commented
        Sep 2, 2022 
      
    
  
- fix: update npm-postinstall.sh script
- fix: use npm in release-standalone
- chore: update package.json
Add --legacy-peer-deps to deal with weird npm issue with vscode dependencies. See: https://stackoverflow.com/a/66620869/3015595
|  | ||
| pushd "$RELEASE_PATH" | ||
| yarn --production --frozen-lockfile | ||
| npm install --unsafe-perm --omit=dev | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That'll help us catch this bug in the future.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry I didn't get to do this yet... It was on my to-do list from #5071 (comment) but hadn't gotten to it yet...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh don't worry! All good 👍🏼 We didn't realize it would cause other issues so no worries at all. At least it's only npm!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I vaguely recall some issue with npm ci and optional dependencies; does that ring any bells?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think those were fixed with the latest versions of NPM, namely NPM 8 which we have since we bumped to NodeJS 16?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ahh gotcha!
| Codecov Report
 Additional details and impacted files@@           Coverage Diff           @@
##             main    #5533   +/-   ##
=======================================
  Coverage   72.44%   72.44%           
=======================================
  Files          30       30           
  Lines        1673     1673           
  Branches      366      366           
=======================================
  Hits         1212     1212           
  Misses        398      398           
  Partials       63       63           Continue to review full report at Codecov. 
 | 
|  | ||
| pushd "$RELEASE_PATH" | ||
| yarn --production --frozen-lockfile | ||
| npm install --unsafe-perm --omit=dev | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry I didn't get to do this yet... It was on my to-do list from #5071 (comment) but hadn't gotten to it yet...
| exit 1 | ||
| else | ||
| npm install --omit=dev | ||
| npm install --unsafe-perm --legacy-peer-deps --omit=dev | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| npm install --unsafe-perm --legacy-peer-deps --omit=dev | |
| # HACK: NPM's use of semver doesn't like resolving some peerDependencies that vscode (upstream) brings in the form of pre-releases. | |
| # The legacy behavior doesn't complain about pre-releases being used, falling back to that for now. | |
| # See https://github.com/coder/code-server/pull/5071 | |
| npm install --unsafe-perm --legacy-peer-deps --omit=dev | 
|  | ||
| pushd "$RELEASE_PATH" | ||
| yarn --production --frozen-lockfile | ||
| npm install --unsafe-perm --omit=dev | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| npm install --unsafe-perm --omit=dev | |
| npm ci --unsafe-perm --omit=dev | 
npm ci is the equivalent of yarn --frozen-lockfile I believe?
| Had the same failure describe in #5530 after installing the with  But cd'ing to  | 
| Nice suggestions! Since it's late afternoon Friday, I think we're going to make those in a followup PR so that we can get these artifacts and fix on npm asap | 
| Ok,  So all good! Also, NPM 8 tip: the logging of nested scripts needs the  (Which BTW,  | 
| 
 Bad new for me the version of the code-server also gives the same error on my side. I'm using a raspberry pi over ipad usb3. | 
| @ravi0the0sun If you install using  | 
| 
 im just posting the error part cuz the entire thing was too damn big i hope this helps Update: after installing  | 
| @edvincent i will make your changes in a followup PR and tag you for review |