Chore/yarn3 #1952
Merged
Chore/yarn3 #1952
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
Contributor
zahnster
commented
Oct 14, 2021
zahnster
commented
- Updates project to Yarn3
- Updates dependencies and scripts to be compatible
|
github-actions
bot
added
Area: Doc Site
|
So the reason yarn install fails is because the "fake" core bundle packages are build artifacts. We don't commit them. So a fresh git clone won't have those package.json files available, so yarn tries to unlink them by removing the entries in the lockfile
reproducible if you run |
Niznikr
reviewed
Oct 15, 2021
There was a problem hiding this comment.
In your Actions you'll want to update the Get yarn cache steps to the following to get the cache correctly:
run: echo "::set-output name=dir::$(yarn config get cacheFolder)"
Niznikr
reviewed
Oct 15, 2021
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. |
|
OK, seems like most things are starting to work. Outstanding issues include:
|
SiTaggart
reviewed
Oct 15, 2021
SiTaggart
reviewed
Oct 15, 2021
| - name: Get yarn cache | ||
| id: yarn-cache | ||
| run: echo "::set-output name=dir::$(yarn cache dir)" | ||
| - name: Get yarn cache directory path |
SiTaggart
reviewed
Oct 15, 2021
|
Test summary
Run details
View run in Cypress Dashboard ➡️ This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard |
SiTaggart
reviewed
Oct 15, 2021
| "packages/paste-utils", | ||
| "packages/paste-website" | ||
| ] | ||
| }, |
There was a problem hiding this comment.
OK, so to prevent yarn thinking the fake @twilio-paste-core packages in the core bundle are real workspaces, we need to not list them as valid workspaces. That means we have to list the valid ones explicitly. But it never considers @twilio-paste-core/x ever again
SiTaggart
reviewed
Oct 15, 2021
SiTaggart
reviewed
Oct 15, 2021
| @@ -3,6 +3,12 @@ | |||
| targetPort = 3000 | |||
| framework = "#auto" | |||
|
|
|||
| [build] | |||
| command = "yarn build && yarn build:theme-designer" | |||
There was a problem hiding this comment.
I was hoping this would take precedent from the admin UI, but it doesn't. Which is weird, because publish directory totally does
|
Definitely seeing some pretty hefty gains from this already 🎉 before: after: |
|
Haven't tried it yet, but this might be the way to cache properly on Netlify. |
github-actions
bot
added
the
Area: Components
|
✔️ Deploy Preview for paste-theme-designer ready! 🔨 Explore the source changes: 77502f2 🔍 Inspect the deploy log: https://app.netlify.com/sites/paste-theme-designer/deploys/6172edd7048e570007d3534b 😎 Browse the preview: https://deploy-preview-1952--paste-theme-designer.netlify.app |
|
✔️ Deploy Preview for paste-docs ready! 🔨 Explore the source changes: 77502f2 🔍 Inspect the deploy log: https://app.netlify.com/sites/paste-docs/deploys/6172edd79c0a06000867d342 😎 Browse the preview: https://deploy-preview-1952--paste-docs.netlify.app |
|
Size Change: +22 B (0%) Total Size: 532 kB
ℹ️ View Unchanged
|
SiTaggart
approved these changes
Oct 22, 2021
shleewhite
approved these changes
Oct 22, 2021
| "types": "./types/index.d.ts", | ||
| "scripts": { | ||
| "bootstrap": "lerna bootstrap", | ||
| "bootstrap": "", |
There was a problem hiding this comment.
Why is this script still here if it does nothing?
There was a problem hiding this comment.
Yeah this is annoying.
There are two ways to set Netlify build commands and settings. In the Netlify admin UI and in the netlify.toml file.
In all but one case, the toml file will take precedent over the Netlify UI settings. All but the build command.
The build command in the Netlify UI includes running yarn bootstrap, which is required for all sites still using the v1 of yarn, but breaks builds using yarn v3.
So to not block everyone from working whilst we did this, but have working builds in this branch, we make sure yarn bootstrap doesn't actually call lerna by blanking out the script in the branch.
Once merged, we'll remove the Netlify UI setting and only use the config files moving forward to prevent this from happening agains.
| "eslint-plugin-react": "^7.20.0", | ||
| "eslint-plugin-react-hooks": "^4.2.0", | ||
| "eslint-plugin-unicorn": "^23.0.0", | ||
| "eslint": "7.23.0", |
There was a problem hiding this comment.
Why did the ^ get removed for all these packages?
There was a problem hiding this comment.
So because we upgrade the lockfile to a completely different format you are effectively doing a completely fresh, unlocked dependency install as if the lockfile doesn't exist. The ^ tells yarn to install the latest version that satisfies the dependency range, but in this case that is a minor dependency bump, the latest v7 of eslint for example.
In those minor bumps some rules have changes for eslint plugins and we get alot of errors. The scope is upgrade yarn, not upgrade eslint, all it's rules and our code to match it.
So we pinned it to keep that scope.
| @@ -30,6 +30,7 @@ | |||
| "@twilio-paste/button": "^7.0.0", | |||
| "@twilio-paste/design-tokens": "^6.6.0", | |||
| "@twilio-paste/flex": "^2.0.2", | |||
| "@twilio-paste/form": "^6.0.2", | |||
There was a problem hiding this comment.
Unrelated question, but why are all the packages listed as both a peer dependency and a dev dependency?
There was a problem hiding this comment.
Good question. It's related to local compilation of packages. I'm sure there is a very good long technical answer, but essentially because the peer dependencies are actually local to the package, if they aren't listed as a dev dependency they are compiled into the package, because the system kind of can't find them. https://github.com/twilio-labs/paste#dependencies
Because they are local, they are symlinked from the root. They are technically dev dependencies to the package when we work and compile, but peer dependencies when they are installed on someone elses computer.
SiTaggart
removed
the
Status: Do Not Merge
SiTaggart
force-pushed
the
chore/yarn3
branch
from
2bcf7ae to
77502f2
Compare
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.