-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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
Replace npm with yarn #5555
Merged
Merged
Replace npm with yarn #5555
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
No longer can reliably access node_modules via thei node_modules directory; yarn may optimize it via hoisting it up. This meant updating JS tasks that were copying files directly from node_modules directories. In these cases, pulled in a new package to resolve these correctly. SCSS files remain impacted, but cannot easily import via JS. These paths have been modified, but it feels dangerous and incorrect to reach into a node_module to grab files like this. Many prebuild steps were removed. I **think** the purpose of `check-deps-pre` is no longer needed, but need to confirm this.
removal of pretest-unit due to check-deps-pre
I do not think these are needed anymore
Replaced by `lerna run` and `lerna run --scope`
appveyor to soon follow
Thanks for the contribution! Below are some guidelines Cypress uses when doing PR reviews.
PR Review ChecklistIf any of the following requirements can't be met, leave a comment in the review selecting 'Request changes', otherwise 'Approve'. User Experience
Functionality
Maintainability
Quality
Internal
|
andrew-codes
force-pushed
the
issue-5477-replace-npm-with-yarn
branch
3 times, most recently
from
October 31, 2019 22:52
7f11968
to
fab87e8
Compare
…me package; mostly mocha. This mocha package will be optimized by yarn workspaces by hoisting it into the root; which is effectively what bin-up usage was mimicing.
also run test in parallel via `yarn test`; it runs test in all 10 packages
yarn does not call postinstall when nothing has installed; such is the case when everything has been cached
find the right node_module dir via `resolve-pkg`
Regarding direct access to node_modules
ignoring check of node/yarn versions when installing
I **think** the reason to not install packages' modules (the reason for `--ignore-scripts` may be irrelevant with yarn workspaces managing the packages)
`--file` was introduce in >=5
`yarn pack` does not output the file name like `npm pack` does. Correct this by explictly setting the filename.
This reverts commit 9256aed.
flotwig
previously approved these changes
Jan 27, 2020
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.
- verified macOS build (Gleb)
- verified windows build runs
- verified linux build
LGTM, nice work @andrew-codes
Co-Authored-By: Zach Bloomquist <[email protected]>
flotwig
approved these changes
Feb 10, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
User facing changelog
N/A
Additional details
Things affected by this change:
yarn test-unit
, etc../scripts
files that became unnecessaryDetails
With yarn workspaces optimizing where node_modules live, we can no longer rely on a node_module existing in a particular node_module directory (root or package level). There were a few places that manually reached into node_modules to read a file, etc. that required updates. One such way to update was to introduce the
resolve-pkg
which a) allows for packages to be resolved within the context of a directory and b) does not require the main field in the resolved packages'package.json
I also updated some root scripts to enable running commands from the root more quickly. New packages should be added in the root and not in the package sub-dir.
How has the user experience changed?
No user experience changes, but developers of cypress will be required to use yarn instead of npm.
PR Tasks
renovate.json
Developer Experience
I think there are two primary use cases to watch out in terms of developer experience; validation and debugging.
Validation means I want to validate my changes, i.e. I want to run tests in a package or packages to verify that my changes work. Validation tasks intentionally execute at critical points in development. It is not necessary to run validation tasks with every change a developer makes.
The second is debugging; running tests and stopping the process for inspection. In this use case, it would be ideal for the debugging process to run at the beginning of my development cycle, with it re-running relevant tests as I change files. However, running tests in watch mode is not sufficient, as in this use case, I also want to stop the running process (breakpoint) to inspect/debug code line by line.
debugging experience via IDE/editor; deferring this to investigate wallabyjs and other possible solutionsUsing Yarn
npm run
withyarn
inpackage.json
scripts-- --arg
; yarn will pass args without the need for--
Using Yarn Workspaces
Maintain
check-deps
functionality. Use case is: a developer changes branches and forgets to re-runyarn
. Check-deps will error out and alert the developer thatyarn
is necessary before running other scripts.Note: building optimizations (only build packages that have changed) are deferred in this PR.
remove unneeded check-depsyarn check --integrity
; see yarn docs for more detailsyarn check --integrity
ever fails.Using Lerna
test-unit
test-integration
test-e2e
watch
prebuild
build
clean-deps
lint-coffee
all
npm task and associated scriptsnpm run all
in rootpackage.json
run.js
and related scriptscircle.yml
appveyor.yml
bin-up
packages and referencescli
lintingrunningnpm run lint
already has errors before the introduction of yarnMeasuring Performance
npm
find . -name node_modules -type d -prune -exec rm -rf {} \; && time npm i
find . -name node_modules -type d -prune -exec rm -rf {} \; && npm i && time npm i
find . -name node_modules -type d -prune -exec rm -rf {} \; && npm i && time npm i react --dev
time npm run build
time npm run all test-unit
du -ch .
yarn
find . -name node_modules -type d -prune -exec rm -rf {} \; && time yarn
find . -name node_modules -type d -prune -exec rm -rf {} \; && yarn && time yarn
find . -name node_modules -type d -prune -exec rm -rf {} \; && yarn && time yarn add -W --dev react
time yarn build
time yarn test-unit
du -ch .
Notes
Removing direct access to node_modules
Node_modules is an opaque structure. We should not reach directly into it to grab files. This issue is highlighted with the introduction of yarn workspaces. Any given node_module may or may not live in its dependent package's node_modules directory. This is because yarn will optimize node_modules; potentially hoisting them into the root of the mono-repo.
This is because the postinstall script attempts to read files within the
@types/blob-util
package. This package has been optimized, hoisted to the root of the repo, and no longer guaranteed to be located in either the package or the root.