test: assert new minified error message on react canary#1199
Conversation
hi-ogawa
changed the title
hi-ogawa
changed the title
There was a problem hiding this comment.
Pull request overview
Updates the plugin-rsc e2e suite and CI React override logic to accommodate React canary/experimental’s newly minified production error message format.
Changes:
- Update e2e assertion to accept the new minified React canary/experimental production error text.
- Unskip react-router e2e suites previously gated by React canary/experimental.
- Replace inline
sedYAML edits in CI with a reusableyq-based script for overriding React packages.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
scripts/override-react.sh |
Adds a helper script to update pnpm-workspace.yaml overrides for React packages via yq. |
packages/plugin-rsc/e2e/react-router.test.ts |
Removes canary/experimental skip gates so these suites run under canary/experimental. |
packages/plugin-rsc/e2e/basic.test.ts |
Adds a canary/experimental-specific assertion for the minified production error message. |
.github/workflows/ci-rsc.yml |
Switches CI React override logic from sed to the new helper script. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| echo "yq is required to update pnpm-workspace.yaml" >&2 | ||
| exit 1 | ||
| fi | ||
|
|
There was a problem hiding this comment.
The script only checks that yq exists, but the expression syntax used here relies on mikefarah/yq v4. If a developer has a different yq implementation (e.g. the Python wrapper), this will fail with a confusing parse error. Consider validating yq --version is v4 (and improving the error message with install/version guidance).
| echo "yq is required to update pnpm-workspace.yaml" >&2 | |
| exit 1 | |
| fi | |
| echo "yq is required to update pnpm-workspace.yaml." >&2 | |
| echo "Please install mikefarah/yq v4: https://github.com/mikefarah/yq/" >&2 | |
| exit 1 | |
| fi | |
| yq_version=$(yq --version 2>&1 || true) | |
| if [[ ! $yq_version =~ version[[:space:]]+v?4\. ]]; then | |
| echo "This script requires mikefarah/yq v4 to update pnpm-workspace.yaml." >&2 | |
| echo "Detected: $yq_version" >&2 | |
| echo "Please install mikefarah/yq v4 and verify with: yq --version" >&2 | |
| exit 1 | |
| fi |
| - name: install react | ||
| if: ${{ matrix.react_version }} | ||
| run: | | ||
| sed -i "/^overrides:/a\ react: \"$REACT_VERSION\"" pnpm-workspace.yaml | ||
| sed -i "/^overrides:/a\ react-dom: \"$REACT_VERSION\"" pnpm-workspace.yaml | ||
| sed -i "/^overrides:/a\ react-server-dom-webpack: \"$REACT_VERSION\"" pnpm-workspace.yaml | ||
| ./scripts/override-react.sh "$REACT_VERSION" | ||
| pnpm i --no-frozen-lockfile |
There was a problem hiding this comment.
This step now depends on ./scripts/override-react.sh, which requires yq, but the workflow never installs/pins yq. Relying on a preinstalled yq on GitHub-hosted runners is brittle (runner images can change) and can break scheduled canary/experimental runs. Consider adding an explicit install/pin step for the expected yq v4 binary on Linux before any yq usage (this also covers the later Vite override step).
Description
There's minor change in a production error message facebook/react#36277. This PR updates assertions to match the new form.