fix(install): Use npm-is to require Yarn before install. Fixes PWA-505. #2384
Conversation
zetlen
added
tooling
|
prod.dockerfile
Outdated
| @@ -23,6 +23,9 @@ COPY packages/venia-concept/package.json ./packages/venia-concept/package.json | |||
| COPY package.json yarn.lock babel.config.js magento-compatibility.js ./ | |||
| COPY scripts/monorepo-introduction.js ./scripts/monorepo-introduction.js | |||
|
|
|||
| # To handle "could not get uid/gid" | |||
| RUN npm config set unsafe-perm true | |||
|
|
|||
There was a problem hiding this comment.
@zetlen On CI yarn install was failing with error https://console.aws.amazon.com/codesuite/codebuild/projects/pwa-pull-request-build/build/pwa-pull-request-build%3A4b8d13c6-fd9e-42d3-83a3-287a61eea4bd/log?region=us-east-1 .
I tried updating to latest docker images but ended up in one or other error. You can check those errors in aws build logs for test pr #2385 commits.
I gave up on that and applied resolution from https://stackoverflow.com/questions/52196518/could-not-get-uid-gid-when-building-node-docker.
Also same image on local works fine bash docker/run-docker so I did not update dev.docker file.
zetlen
force-pushed
the
zetlen/require-yarn-at-preinstall
branch
from
163304b to
31247ba
Compare
package.json
Outdated
| @@ -30,6 +30,7 @@ | |||
| "danger": "danger-ci", | |||
| "lint": "eslint '@(packages|scripts)/**/{*.js,package.json}' --ignore-pattern node_modules --ignore-pattern storybook-dist", | |||
| "postbuild": "rimraf \"./packages/*/dist/{,**/}__*__\"", | |||
| "preinstall": "node -e 'process.env.CI||process.exit(1)' || npx npm-is npm", | |||
There was a problem hiding this comment.
Makes sense to me. Does npm-is not need to be added to the list of dependencies?
There was a problem hiding this comment.
@zetlen I think you meant node -e 'process.env.CI||process.exit(1)' || npx npm-is yarn?
There was a problem hiding this comment.
@dpatil-magento I did, sorry. I was testing it and forgot to put it back.
@jimbo It does not! That’s why we use npx to run it, because it has to work before any dependencies are installed.
| @@ -9,6 +9,9 @@ RUN apk --no-cache --virtual add \ | |||
| g++ \ | |||
| yarn | |||
|
|
|||
| # set env variable for CI | |||
| ENV CI=true | |||
|
|
|||
There was a problem hiding this comment.
AWS has CI env variable but dockerfile needs to have also.
Description
Add a preinstall script that prevents
npm install, requiringyarn installinstead.The monorepo won't work right if installed with
npm. Several users have reported thatnpm installat repo root causes somewhat mysterious errors. This is because the repo uses Yarn Workspaces, and Yarn automatically installs workspace dependencies along with root dependencies. NPM doesn't, so packages are missing deps when they run.I, uh, found a utility called
npm-iswhich detects and asserts package managers, and used it to add a preinstall scriptnpx npm-is yarn. This command fails if the NPM script wasn't invoked with Yarn, thus preventingnpm install.Related Issue
Closes PWA-505.
Acceptance
Verification Stakeholders
@dpatil-magento
Verification Steps
yarn run clean:allto remove all node_modules folders.npm installat package root.Checklist