-
Notifications
You must be signed in to change notification settings - Fork 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
Trying out yarn #39121
Trying out yarn #39121
Conversation
While running The |
Here is how your PR affects size of JS and CSS bundles shipped to the user's browser: App Entrypoints (~18 bytes removed 📉 [gzipped])
Common code that is always downloaded and parsed every time the app is loaded, no matter which route is used. Sections (~4819 bytes removed 📉 [gzipped])
Sections contain code specific for a given set of routes. Is downloaded and parsed only when a particular route is navigated to. Async-loaded Components (~83040 bytes added 📈 [gzipped])
React components that are loaded lazily, when a certain part of UI is displayed for the first time. Legend What is parsed and gzip size?Parsed Size: Uncompressed size of the JS and CSS files. This much code needs to be parsed and stored in memory. Generated by performance advisor bot at iscalypsofastyet.com. |
client/package.json
Outdated
@@ -119,7 +119,7 @@ | |||
"path-browserify": "1.0.0", | |||
"percentage-regex": "3.0.0", | |||
"phone": "2.4.2", | |||
"photon": "file:../packages/photon", | |||
"photon": "^3.0.0", |
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.
We probably want to use ^
syntax everywhere there was a file:
reference - otherwise everytime a local package is bumped we'll have to go and update all the references to it so yarn
continues to symlink the workspace instead of installing the specific version from the registry.
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.
🤔 what happens if local version is bumped v4 in this case? Would we then want to use *
instead?
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.
Yeah for private packages like client/*
it might be a good idea to encourage the developer who makes a change to also update any usages across the codebase.
In packages published to the registry, *
would have unintended affects.
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.
This is a very tricky subject. If we don't force version sync across all the monrepo (ie. update all usages of a package when it changes version), we may end up in this scenario:
./packages/packageA
depends on[email protected]
./packages/packageB
is bumped to 2.0.0, with breaking changes- Dependency in
./packages/packageA
is not updated.
That will make ./packages/packageA
code incompatible with ./packages/packageB
, which will be a source of confusion and a pain to debug.
Also, consumers of packageA
and packageB
may end up with two copies: [email protected] (because trans dep from packageA) and [email protected] (because they are on the latest version).
One of the downsides is that updating a package will have a "cascade" effect that may end up updating a ton of packages, therefore requiring a lot of releases. And by extension, adding extra work for consumers of our packages if they want to be up to date.
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'm all for adding a version constraint that enforces workspaces are always symlinked (the goal of adding ^
was to keep it symlinked for longer). I tried yarn@2
with its constraints in mind but the ecosystem isn't stable enough yet. Maybe we can use manypkg
to enforce that for now?
The remaining CI failure:
Is due to Converting the paths to use |
|
443b52a
to
66d84cf
Compare
fse just moved to dotcom-fse |
48dc7be
to
7522e25
Compare
rebased this and force-pushed it @jameslnewell |
e90f00e
to
5a9aebb
Compare
ac0401e
to
a6071b9
Compare
Inital moves to yarn workspaces - add workspaces to root package.json - move to actual semver versions for monorepo packages instead of file: refs replaced file: with local package version, updated npm run and circleci update docker ignore engines fix install moved wpcom.js remove npm lock file, fix deps file, update wpcom dep use ^ wherever there was a file: reference update @automattic/color-studio to version that doesn't spec engines resolve a single instance of @emotion/core remove --ignore-engines update lerna config to teach it about yarn and workspaces add e2e tests to workspaces cleanup after rebase updated @wordpress/jest-preset-default resolve delete conflicts Inital moves to yarn workspaces - add workspaces to root package.json - move to actual semver versions for monorepo packages instead of file: refs replaced file: with local package version, updated npm run and circleci ignore engines moved wpcom.js remove npm lock file, fix deps file, update wpcom dep update @automattic/color-studio to version that doesn't spec engines resolve a single instance of @emotion/core remove --ignore-engines add e2e tests to workspaces cleanup after rebase updated @wordpress/jest-preset-default updated lock use immutable instead of frozen-lockfile remove package-lock.json from lerna ignores remove unnecessary -- when running typecheck spec the yarn version we want tell renovate to run yarn-deduplicate --fewer after each update run loosen yarn version to match up with what circle ci has npm->yarn for various steps and tasks move back to --frozen-lockfile, --immutable is a 2.0 thing Update docs/lockfile.md Co-Authored-By: Marin Atanasov <[email protected]> Update docs/monorepo.md Co-Authored-By: Marin Atanasov <[email protected]> update lockfile docs update install script fix grammar in lockfile docs remove redundant -- update npm text and commands in various locations fix missed conflicts bring back package-lock.json for e2e tests cleanup after rebase update new references to npm to use yarn fix file: reference fix lock file and usage of npm resolve conflict fix typings issues after rebase due to multiple versions of a package
…to be on the root
ce82ff1
to
673880f
Compare
Merged as part of #41140 |
Try out yarn!
yarn --ignore-engines
)update(will do this in a future PR to keep this PR readable)peerDependency
warnings - this probably involve adding the necessarydevDependencies
to each of the workspaces--ignore-engines
decide if we want to enforceyarn-deduplicate