[release-4.22] CONSOLE-5011: migrate to yarn berry#15986
[release-4.22] CONSOLE-5011: migrate to yarn berry#15986logonoff wants to merge 2 commits intoopenshift:mainfrom
Conversation
openshift-ci
bot
added
the
do-not-merge/hold
openshift-ci
bot
added
component/core
logonoff
changed the title
logonoff
force-pushed
the
yarn4
branch
9 times, most recently
from
5d42266 to
906d4b4
Compare
openshift-ci
bot
added
component/helm
logonoff
force-pushed
the
yarn4
branch
3 times, most recently
from
9ffb88d to
2f6636b
Compare
logonoff
changed the title
openshift-ci-robot
added
the
jira/valid-reference
|
@logonoff: This pull request references CONSOLE-5011 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.22.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
logonoff
force-pushed
the
yarn4
branch
2 times, most recently
from
9f3f11c to
89b3ba2
Compare
📝 WalkthroughWalkthroughThis pull request upgrades the project from Yarn v1 (Classic) to Yarn v4 (Berry) and introduces Corepack as the package manager version manager. Changes include: updating configuration files to use 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
|
/unhold |
|
/retest |
openshift-ci
bot
removed
the
do-not-merge/hold
logonoff
force-pushed
the
yarn4
branch
2 times, most recently
from
d91116b to
c71ff67
Compare
| npm install --global https://github.com/nodejs/corepack/releases/download/v${COREPACK_VERSION}/corepack.tgz; \ | ||
| fi | ||
|
|
||
| RUN npx corepack enable |
There was a problem hiding this comment.
By default, Corepack will setup all supported package managers - we just need Yarn.
| RUN npx corepack enable | |
| RUN npx corepack enable yarn |
There was a problem hiding this comment.
I was hoping by making it generic we won't need to change this again for when we want pnpm
There was a problem hiding this comment.
Good point, in that case we can keep it generic.
There was a problem hiding this comment.
Is this link really needed? Why not just update demo plugin .yarnrc.yml
yarnPath: ../frontend/.yarn/releases/yarn-4.12.0.cjsThere was a problem hiding this comment.
This is to reduce config file duplication of .yarnrc.yml
| "build": "yarn clean && yarn validate && yarn compile && yarn generate", | ||
| "compile": "for ext in '' '-internal' '-webpack' ; do ../../node_modules/.bin/tsc -p tsconfig${ext}.json || exit $? ; done", | ||
| "generate": "yarn generate-schema && yarn generate-doc && yarn generate-pkg-assets", | ||
| "compile": "yarn compile-core & yarn compile-internal & yarn compile-webpack", |
There was a problem hiding this comment.
Using & this way looks very unintuitive to me, it's one of these Yarn weird things / best practices 😄
This deviates from the usual expectation in languages like Bash (and many others, including JavaScript etc.) where && relates to how we handle left-hand vs. right-hand expression, which has no relation to parallelism.
In general, I don't like Yarn imposing the rule "don't use system shell scripts in your package.json scripts because <insert Yarn maintainer's poem about why it's a bad thing>" but at the same time I understand that we do things how Yarn expects them to avoid issues.
| process.exit(1); | ||
| } | ||
| const lockFileContent = readFileSync('yarn.lock', 'utf8'); | ||
| const lockFile = parseSyml(lockFileContent); |
There was a problem hiding this comment.
Should we keep the code that checks if lock file was parsed successfully?
There was a problem hiding this comment.
Error is thrown by the parser library now
// frontend/node_modules/@yarnpkg/parsers/lib/syml.js
if (typeof value !== `object`)
throw new Error(`Expected an indexed object, got a ${typeof value} instead. Does your file follow Yaml's rules?`);
if (Array.isArray(value))
throw new Error(`Expected an indexed object, got an array instead. Does your file follow Yaml's rules?`);
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: logonoff, rhamilto, vojtechszocs The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/label plugin-api-approved |
openshift-ci
bot
added
the
plugin-api-approved
|
@logonoff: This pull request references CONSOLE-5011 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target only the "4.22.0" version, but multiple target versions were set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
New changes are detected. LGTM label has been removed. |
|
/verified by CI I additionally did these steps for testing:
|
openshift-ci-robot
added
the
verified
|
@logonoff: This PR has been marked as verified by DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@logonoff: This pull request references CONSOLE-5011 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target only the "4.22.0" version, but multiple target versions were set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@frontend/package.json`:
- Line 60: The "generate" npm script currently runs "yarn generate-graphql &
yarn build-plugin-sdk" which launches both tasks in the background and returns
immediately, causing a race with the webpack build; change the "generate" script
to run the tasks sequentially so generation finishes before build (for example
replace the command with "yarn generate-graphql && yarn build-plugin-sdk") so
that the "generate" script (and downstream "build" that depends on it) always
sees completed generated types/SDK artifacts.
In `@frontend/packages/console-dynamic-plugin-sdk/package.json`:
- Around line 10-14: The scripts currently background multiple processes (using
&), causing the parent script to exit before children finish; update the
"compile" script to run the three tsc commands in background and then block
until they finish (e.g., append a wait after "yarn compile-core & yarn
compile-internal & yarn compile-webpack") and update the "generate" script to
run "yarn generate-schema && yarn generate-doc & yarn generate-pkg-assets" but
ensure the backgrounded generate-doc is followed by a wait so "generate" only
returns after generate-doc completes; refer to the "compile", "compile-core",
"compile-internal", "compile-webpack", "generate", "generate-doc", and
"generate-pkg-assets" script entries when making the changes.
🧹 Nitpick comments (1)
README.md (1)
421-423: Add language specifier to the fenced code block.Static analysis flagged this code block as missing a language identifier. Adding
shorbashimproves syntax highlighting and linter compliance.📝 Proposed fix
-``` +```sh yarn dedupe --strategy highest</details> </blockquote></details> </blockquote></details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
| "prettier-all": "prettier --write '**/*.{js,jsx,ts,tsx,json}'", | ||
| "ts-node": "ts-node -O '{\"module\":\"commonjs\"}'", | ||
| "generate": "yarn generate-graphql && yarn build-plugin-sdk", | ||
| "generate": "yarn generate-graphql & yarn build-plugin-sdk", |
There was a problem hiding this comment.
Parallel generate script lacks wait - may cause race with webpack build.
Same issue as the SDK package: yarn generate-graphql & yarn build-plugin-sdk forks both jobs and returns immediately. Since build (line 19) chains yarn generate && ... webpack, webpack may start before codegen/SDK generation completes, leading to missing generated types or stale SDK artifacts.
🐛 Proposed fix
- "generate": "yarn generate-graphql & yarn build-plugin-sdk",
+ "generate": "yarn generate-graphql & yarn build-plugin-sdk & wait",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "generate": "yarn generate-graphql & yarn build-plugin-sdk", | |
| "generate": "yarn generate-graphql & yarn build-plugin-sdk & wait", |
🤖 Prompt for AI Agents
In `@frontend/package.json` at line 60, The "generate" npm script currently runs
"yarn generate-graphql & yarn build-plugin-sdk" which launches both tasks in the
background and returns immediately, causing a race with the webpack build;
change the "generate" script to run the tasks sequentially so generation
finishes before build (for example replace the command with "yarn
generate-graphql && yarn build-plugin-sdk") so that the "generate" script (and
downstream "build" that depends on it) always sees completed generated types/SDK
artifacts.
| "compile": "yarn compile-core & yarn compile-internal & yarn compile-webpack", | ||
| "compile-core": "../../node_modules/.bin/tsc -p tsconfig.json", | ||
| "compile-internal": "../../node_modules/.bin/tsc -p tsconfig-internal.json", | ||
| "compile-webpack": "../../node_modules/.bin/tsc -p tsconfig-webpack.json", | ||
| "generate": "yarn generate-schema && yarn generate-doc & yarn generate-pkg-assets", |
There was a problem hiding this comment.
Missing wait causes premature script completion - builds may be non-deterministic.
Using & to fork background jobs without a trailing wait means the script returns immediately, not when the jobs complete. This breaks the build script's yarn compile && yarn generate chain because compile returns before TypeScript actually finishes.
Similarly, line 14's generate-doc & runs in the background with no wait, so generate may complete before doc generation finishes.
🐛 Proposed fix - add `wait` to ensure completion
- "compile": "yarn compile-core & yarn compile-internal & yarn compile-webpack",
+ "compile": "yarn compile-core & yarn compile-internal & yarn compile-webpack & wait",
...
- "generate": "yarn generate-schema && yarn generate-doc & yarn generate-pkg-assets",
+ "generate": "yarn generate-schema && yarn generate-doc & yarn generate-pkg-assets & wait",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "compile": "yarn compile-core & yarn compile-internal & yarn compile-webpack", | |
| "compile-core": "../../node_modules/.bin/tsc -p tsconfig.json", | |
| "compile-internal": "../../node_modules/.bin/tsc -p tsconfig-internal.json", | |
| "compile-webpack": "../../node_modules/.bin/tsc -p tsconfig-webpack.json", | |
| "generate": "yarn generate-schema && yarn generate-doc & yarn generate-pkg-assets", | |
| "compile": "yarn compile-core & yarn compile-internal & yarn compile-webpack & wait", | |
| "compile-core": "../../node_modules/.bin/tsc -p tsconfig.json", | |
| "compile-internal": "../../node_modules/.bin/tsc -p tsconfig-internal.json", | |
| "compile-webpack": "../../node_modules/.bin/tsc -p tsconfig-webpack.json", | |
| "generate": "yarn generate-schema && yarn generate-doc & yarn generate-pkg-assets & wait", |
🤖 Prompt for AI Agents
In `@frontend/packages/console-dynamic-plugin-sdk/package.json` around lines 10 -
14, The scripts currently background multiple processes (using &), causing the
parent script to exit before children finish; update the "compile" script to run
the three tsc commands in background and then block until they finish (e.g.,
append a wait after "yarn compile-core & yarn compile-internal & yarn
compile-webpack") and update the "generate" script to run "yarn generate-schema
&& yarn generate-doc & yarn generate-pkg-assets" but ensure the backgrounded
generate-doc is followed by a wait so "generate" only returns after generate-doc
completes; refer to the "compile", "compile-core", "compile-internal",
"compile-webpack", "generate", "generate-doc", and "generate-pkg-assets" script
entries when making the changes.
|
@logonoff: This pull request references CONSOLE-5011 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target only the "4.22.0" version, but multiple target versions were set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
@logonoff: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
6 participants
This PR (and its "backports" all the way to 4.12) aim to perform the mandatory migration from yarn classic to yarn berry (of the v4 variety).
Changes
See #15995 for some preparatory work that was done from 4.12 to 4.18.
Note: the list of changes are descriptive for all backport PRs. Some of these changes may not apply to every backport.
Dockerfiles to install corepack instead of yarn v1dynamic-demo-plugin, symlink.yarnrc.ymland the.yarn/releasesfolder to reduce config duplicationpackage.jsonscripts to have syntax compatible with yarn berrycheck-patternfly-modulesscript to use new yarn berry parsing packageyarn dedupestuff with the yarn berry-provided versionportalinstead offilenowupdate-patternfly.shbecause PatternFly no longer consistently has all libraries to the same version.yarn/releasesto point to yarn v4. Note that this is for installations (e.g.,tectonic-console-builder:v29) which already have yarn classic installed globally. They will read this updated binary and automatically run yarn berry. We can remove this when our builder image switches fully over to corepackTest cases
scriptsinpackage.jsonstill work finebuild.sh, etc.) work fine on an initial repo state, switching branches, etc..gitfolder infrontendis created everBackports
TBD
Summary by CodeRabbit
Release Notes
New Features
Chores
Documentation