RFC: Tighten dependencies for Shakapacker v11 - #1096
Conversation
Proposes simplifying the dependency surface by tightening version ranges, dropping EOL runtime support, and clearly separating managed build vs custom build modes. Reduces peer deps from 23 to ~16 and version ranges from 50+ to ~20. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Incorporates community feedback from #1030: - Split into three npm packages (core, @shakapacker/webpack, @shakapacker/rspack) instead of conditional peer deps in a single package - Keep @types/* as optional peer deps (used by exported types) - Don't raise minor version floors without cause - Fix Vite/Rolldown characterization Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Resolved open question: use unscoped names instead of @shakapacker/* scoped packages to avoid npm org setup requirement. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- All three npm packages live in existing shakacode/shakapacker repo - Lockstep versioning: all packages always release at the same version - Start at 17.0.0-beta.1 - Supplemental packages use caret peer dep on core (^17.0.0) - Added "separate repos" to alternatives considered (rejected) - Moved resolved decisions to their own section Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Key changes: - Phase 1 (v10.1.0): additive, publish supplemental packages, keep existing peer deps in core, zero-risk for existing users - Phase 2 (v11.0.0): breaking, remove peer deps from core, tighten ranges, drop EOL runtimes - All npm packages under packages/ (not package/ + packages/) - Version targets updated from v17 to v10.1/v11 (v10 just shipped) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Adds shakapacker-webpack and shakapacker-rspack under packages/ as thin supplemental packages that re-export from the core shakapacker package. Core package/ directory stays in place for v10.1.0 — moving it under packages/shakapacker/ is deferred to v11.0.0 to avoid breaking deep imports. No peer deps removed from core; this is additive and non-breaking. Updates the RFC to reflect that Phase 1 keeps core in place, with separate structure diagrams for Phase 1 (v10.1.0) and Phase 2 (v11.0.0). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds two new lockstep npm packages (shakapacker-webpack, shakapacker-rspack) with manifests, entrypoints, and license files; introduces an RFC for phased dependency tightening; updates devDependencies and Node pin; and adds a publish orchestrator script and runtime transpiler warning in the webpack entrypoint. ChangesMonorepo packaging, entrypoints, and publish flow
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Code ReviewOverviewThis PR adds an RFC proposing a 3-package npm monorepo split for Shakapacker (core + What's Working Well
Issues🔴 Breaking: Peer dep version ahead of published coreBoth supplemental 🟡 Filename / version naming mismatchThe RFC file is named 🟡 Missing
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
Greptile SummaryThis PR adds an RFC proposing a phased monorepo split of Shakapacker into three npm packages (
Confidence Score: 4/5Safe to merge for discussion purposes, but the supplemental package scaffolds have a broken peer dep version that must be resolved before v10.1.0 is published. One P1 finding: the new packages declare a peer dep on shakapacker@^10.1.0 which doesn't exist yet, making them non-installable if accidentally published. The version label mismatch (v17 vs v10.1/v11) and missing engines field are P2 quality issues that don't block merge but should be cleaned up. packages/shakapacker-webpack/package.json and packages/shakapacker-rspack/package.json — both reference a non-existent shakapacker@10.1.0 peer dep and are missing an engines field. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
subgraph v10["Phase 1: v10.1.0 (non-breaking)"]
core["shakapacker (core)\n9.x peer deps unchanged"]
wpkg["shakapacker-webpack\nnew supplemental pkg"]
rpkg["shakapacker-rspack\nnew supplemental pkg"]
end
subgraph v11["Phase 2: v11.0.0 (breaking)"]
core2["shakapacker (core)\nno bundler peer deps"]
wpkg2["shakapacker-webpack\nrequired for webpack users"]
rpkg2["shakapacker-rspack\nrequired for rspack users"]
end
user_wp["Webpack user"] --> wpkg --> core
user_rp["Rspack user"] --> rpkg --> core
user_custom["Custom build user"] --> core
v10 -->|"breaking: remove bundler deps from core"| v11
wpkg2 -->|"peer dep ^11.0.0"| core2
rpkg2 -->|"peer dep ^11.0.0"| core2
Reviews (1): Last reviewed commit: "Implement Phase 1 supplemental packages ..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
docs/rfcs/0001-tighten-dependencies-v17.md (2)
244-281: Nit: specify a language on the tree-diagram fenced blocks.markdownlint MD040 flags the two directory-tree code fences (lines 244 and 264) for missing a language. Use
text(orplaintext) to silence the warning and keep docs lint-clean.✏️ Proposed fix
-``` +```text shakapacker/ ├── package/ # core npm package source (unchanged)(apply the same change to the second tree at line 264)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/rfcs/0001-tighten-dependencies-v17.md` around lines 244 - 281, Update the two directory-tree fenced code blocks in docs/rfcs/0001-tighten-dependencies-v17.md (the blocks showing the shakapacker/ tree in Phase 1 and Phase 2) to include a language identifier (use "text" or "plaintext") after the opening triple backticks so the blocks become ```text ... ``` to satisfy markdownlint MD040.
3-5: Stale title/status metadata.The file is
0001-tighten-dependencies-v17.mdand the H1 still says "Tighten Dependencies" (fine), but the status line reads "Rev 5 — phased rollout v10.1 / v11" while the filename and PR title still reference v17. Consider renaming the file to match the phased v10.1/v11 plan (e.g.,0001-tighten-dependencies-phased.md) so future contributors aren't confused by the v17 label.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/rfcs/0001-tighten-dependencies-v17.md` around lines 3 - 5, The RFC file name and its "Status:" metadata are inconsistent (file 0001-tighten-dependencies-v17.md vs Status: "Rev 5 — phased rollout v10.1 / v11"); either rename the file to reflect the phased rollout (suggestion: 0001-tighten-dependencies-phased.md) or update the Status line to match v17, and ensure the PR title/branch name and any internal references use the new name; update any links or table-of-contents entries that reference 0001-tighten-dependencies-v17.md so everything remains consistent.packages/shakapacker-rspack/package.json (1)
19-28: Peer dep floor forshakapackershould allow the same-version co-install.
"shakapacker": "^10.1.0"requires users to have core at ≥10.1.0, which is fine for Phase 1. However, under the RFC's lockstep rule ("all three npm packages share the same version number"), you'll likely want to re-examine this range at each bump — in particular, when v11 lands, this must be updated to^11.0.0to prevent a v10 core + v11 supplemental pairing. Consider adding a release-script check that asserts the core peer range matches the package's own major.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/shakapacker-rspack/package.json` around lines 19 - 28, The peerDependencies entry for "shakapacker" in packages/shakapacker-rspack/package.json uses a hardcoded range "^10.1.0" which will diverge when the project bumps majors; update the "shakapacker" peer range to match the package's own major (e.g., use a caret range that references the current major like "^<currentMajor>.0.0") and add a release-time validation script that runs during publish/release (hooked from package.json scripts) to assert that the "shakapacker" peerDependency major equals this package's package.json "version" major — locate and modify the peerDependencies block and add a small check script (or CI step) that extracts and compares the majors of package.json.version and peerDependencies["shakapacker"] to fail the release if they differ.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/shakapacker-rspack/index.js`:
- Line 1: The file exports the module with a single line module.exports =
require("shakapacker/rspack"); but is missing a trailing newline which fails
Prettier --check; fix by adding a terminating newline character at the end of
the file (preserve the existing statement and its semicolon/semicolon-free style
to match the repo Prettier config) so the file ends with a newline.
In `@packages/shakapacker-rspack/package.json`:
- Around line 21-24: The package.json declares `@rspack/core` and `@rspack/cli` as
^2.0.0 which will fail install because v2 GA isn't published; update the
dependency spec for the symbols "@rspack/core" and "@rspack/cli" in package.json
to a published stable range such as ^1.7.0 (or pin to the exact latest stable
1.7.11) so installs succeed, or alternatively add a clear comment/README note
documenting that v2.0.0 is a pre-release and that the project requires accepting
pre-release versions until the GA is published.
In `@packages/shakapacker-webpack/index.js`:
- Line 1: The proxy module currently does require("shakapacker/webpack") which
will fail because package/webpack/ is missing; fix by either (A) adding a new
package/webpack/index.ts that mirrors package/rspack/index.ts and re-exports the
webpack-specific API (ensure it compiles to package/webpack/index.js and is
included in the package exports), or (B) as a temporary Phase 1 change, update
the proxy to require("shakapacker") instead of "shakapacker/webpack" so
consumers get the existing root exports until you add the dedicated
package/webpack entrypoint; choose one option and ensure package.json exports
map and built artifacts align with the chosen approach.
---
Nitpick comments:
In `@docs/rfcs/0001-tighten-dependencies-v17.md`:
- Around line 244-281: Update the two directory-tree fenced code blocks in
docs/rfcs/0001-tighten-dependencies-v17.md (the blocks showing the shakapacker/
tree in Phase 1 and Phase 2) to include a language identifier (use "text" or
"plaintext") after the opening triple backticks so the blocks become ```text ...
``` to satisfy markdownlint MD040.
- Around line 3-5: The RFC file name and its "Status:" metadata are inconsistent
(file 0001-tighten-dependencies-v17.md vs Status: "Rev 5 — phased rollout v10.1
/ v11"); either rename the file to reflect the phased rollout (suggestion:
0001-tighten-dependencies-phased.md) or update the Status line to match v17, and
ensure the PR title/branch name and any internal references use the new name;
update any links or table-of-contents entries that reference
0001-tighten-dependencies-v17.md so everything remains consistent.
In `@packages/shakapacker-rspack/package.json`:
- Around line 19-28: The peerDependencies entry for "shakapacker" in
packages/shakapacker-rspack/package.json uses a hardcoded range "^10.1.0" which
will diverge when the project bumps majors; update the "shakapacker" peer range
to match the package's own major (e.g., use a caret range that references the
current major like "^<currentMajor>.0.0") and add a release-time validation
script that runs during publish/release (hooked from package.json scripts) to
assert that the "shakapacker" peerDependency major equals this package's
package.json "version" major — locate and modify the peerDependencies block and
add a small check script (or CI step) that extracts and compares the majors of
package.json.version and peerDependencies["shakapacker"] to fail the release if
they differ.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 69729207-f320-4a27-9a54-e5deb74022f0
📒 Files selected for processing (5)
docs/rfcs/0001-tighten-dependencies-v17.mdpackages/shakapacker-rspack/index.jspackages/shakapacker-rspack/package.jsonpackages/shakapacker-webpack/index.jspackages/shakapacker-webpack/package.json
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: faee9d3293
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
- Fix webpack proxy: point to "shakapacker" (root entry) instead of
"shakapacker/webpack" since package/webpack/ does not exist yet.
The rspack proxy stays on "shakapacker/rspack" because package/rspack/
is a real subpath. Splitting webpack into its own subpath is a
Phase 2 concern per the RFC.
- Remove trailing semicolons from both index.js files to satisfy
prettier (repo config is semi: false).
- Relax peer shakapacker range from ^10.1.0 to ^10.0.0 so the
supplemental packages are installable against the currently
published core.
- Drop @rspack/core and @rspack/cli from ^2.0.0 to
^1.0.0 || ^2.0.0-0 to match the root package.json range (no
@rspack/core@2 GA is published yet).
- Widen swc-loader to ^0.1.15 || ^0.2.0 to match root package.json.
- Tighten @swc/core from ^1.3.0 to ^1.10.0 for better error messages
on unsupported SWC builds.
- Add engines { node: ">= 20", yarn: ">=1 <5" } to both supplemental
package.json files so npm/yarn warn on incompatible runtimes.
- Rename docs/rfcs/0001-tighten-dependencies-v17.md to
0001-tighten-dependencies-v11.md to match the RFC body, which
consistently targets v10.1 / v11.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Thanks — addressed the actionable findings in bffd1d3:
Lockstep version inconsistency (core at |
justin808
changed the title
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
docs/rfcs/0001-tighten-dependencies-v11.md (1)
244-244: Consider specifying language for fenced code blocks.The directory structure code blocks at lines 244 and 264 are flagged by markdownlint for missing language specifiers. While these are ASCII art directory trees (not executable code), adding
```textinstead of```would satisfy the linter and improve rendering consistency.📋 Proposed fix
-``` +```text shakapacker/ ├── package/ # core npm package source (unchanged)Apply the same change at line 264.
Also applies to: 264-264
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/rfcs/0001-tighten-dependencies-v11.md` at line 244, The fenced ASCII-dir blocks at the locations around lines noted use plain triple backticks; update those fenced code blocks (the directory tree examples in this RFC) to specify a language by changing the opening fence from ``` to ```text for both the block starting near the first directory tree (around the block that begins with "shakapacker/") and the similar block later (the one around line 264) so markdownlint stops flagging missing language specifiers.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/rfcs/0001-tighten-dependencies-v11.md`:
- Around line 1-19: Summary: The RFC filename and contents reference v11/v10.1.0
but the PR title and description reference v17, causing a version mismatch. Fix:
decide whether the target is v11 or v17 and make the PR metadata and RFC
consistent; if v11 is correct (per commit message), update the PR title and
description to reference v11 and replace any `v17` mentions in PR objectives and
description, otherwise update the RFC file name
`0001-tighten-dependencies-v11.md` and all in-file version tokens (`v10.1.0`,
`v11.0.0`) to `v17`/`v17.x` as appropriate; verify the commit message and branch
name match the chosen version and update any other references in the PR body or
files accordingly.
- Around line 43-46: Summary: The RFC incorrectly states Next.js has 4 peer
dependencies; it currently has 6. Update the paragraph that begins "Next.js has
only 4 peer dependencies (react, react-dom, sass, `@opentelemetry/api` ...)" to
reflect 6 peers by adding the two missing packages: `@playwright/test` and
babel-plugin-react-compiler, and adjust the parenthetical text accordingly so
the list reads: react, react-dom, sass, `@opentelemetry/api`, `@playwright/test`,
babel-plugin-react-compiler; keep the rest of the comparison text intact.
- Around line 118-181: The peer dependency tables under the
`shakapacker-webpack` and `shakapacker-rspack` sections are ambiguous about
which release phase they describe (they list "shakapacker: ^10.1.0" while later
examples use ^11.0.0); update these tables to either use `^11.0.0` if they
represent the Phase 2 final state, or add explicit phase headings (e.g., "Phase
1 (v10.1.0)" and "Phase 2 (v11.0.0)") and duplicate/adjust the peer dependency
rows under each phase so the evolution of `shakapacker` and other peer deps is
clear for `shakapacker-webpack` and `shakapacker-rspack`.
---
Nitpick comments:
In `@docs/rfcs/0001-tighten-dependencies-v11.md`:
- Line 244: The fenced ASCII-dir blocks at the locations around lines noted use
plain triple backticks; update those fenced code blocks (the directory tree
examples in this RFC) to specify a language by changing the opening fence from
``` to ```text for both the block starting near the first directory tree (around
the block that begins with "shakapacker/") and the similar block later (the one
around line 264) so markdownlint stops flagging missing language specifiers.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 189471bd-87c8-4ec1-bd7d-1270f908cfff
📒 Files selected for processing (5)
docs/rfcs/0001-tighten-dependencies-v11.mdpackages/shakapacker-rspack/index.jspackages/shakapacker-rspack/package.jsonpackages/shakapacker-webpack/index.jspackages/shakapacker-webpack/package.json
✅ Files skipped from review due to trivial changes (3)
- packages/shakapacker-rspack/index.js
- packages/shakapacker-webpack/package.json
- packages/shakapacker-rspack/package.json
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/shakapacker-webpack/index.js
Code Review — RFC: Tighten Dependencies (PR #1096)OverviewThis PR introduces a well-reasoned RFC for splitting Shakapacker into three npm packages ( Issues to Fix
Pre-release suffix in rspack peer deps
Root Observations / Discussion PointsTitle mismatch: PR title says "v17", RFC file name says All transpilers optional with no "preferred" signal: In No README in either supplemental package: When these are published to npm, the package page will be blank. A one-paragraph README explaining what the package is, what peer deps are required vs optional, and how it relates to core No CHANGELOG entry: New published npm packages are user-visible. Per project convention,
RFC open questions: Five questions remain open. Before cutting v10.1.0, at minimum Q2 (upgrade task) and Q5 (workspace tooling — npm vs yarn vs changesets) need decisions, as they affect the release process. The others could remain open for v11 discussion. SummaryThe design is solid and the phased rollout is the right call. Fix the peer dep floors, the rspack pre-release range, the |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Autofix Details
Bugbot Autofix prepared fixes for both issues found in the latest run.
- ✅ Fixed: Webpack package re-exports wrong module path
- Updated
packages/shakapacker-webpack/index.jsto re-exportshakapacker/webpackso the supplemental webpack package points to the webpack-specific module.
- Updated
- ✅ Fixed: SWC version floor contradicts RFC and core
- Lowered
@swc/coreinpackages/shakapacker-webpack/package.jsonfrom^1.10.0to^1.3.0to match the RFC and core package peer floor.
- Lowered
Or push these changes by commenting:
@cursor push b3e83d0ae6
Preview (b3e83d0ae6)
diff --git a/packages/shakapacker-webpack/index.js b/packages/shakapacker-webpack/index.js
--- a/packages/shakapacker-webpack/index.js
+++ b/packages/shakapacker-webpack/index.js
@@ -1 +1 @@
-module.exports = require("shakapacker")
+module.exports = require("shakapacker/webpack")
diff --git a/packages/shakapacker-webpack/package.json b/packages/shakapacker-webpack/package.json
--- a/packages/shakapacker-webpack/package.json
+++ b/packages/shakapacker-webpack/package.json
@@ -26,7 +26,7 @@
"terser-webpack-plugin": "^5.3.1",
"webpack-subresource-integrity": "^5.1.0",
"@pmmmwh/react-refresh-webpack-plugin": "^0.5.0",
- "@swc/core": "^1.10.0",
+ "@swc/core": "^1.3.0",
"swc-loader": "^0.1.15 || ^0.2.0",
"@babel/core": "^7.17.9",
"babel-loader": "^9.0.0",You can send follow-ups to the cloud agent here.
| ]) | ||
| ) | ||
| }) | ||
| }) |
There was a problem hiding this comment.
Missing test: the webpack wrapper's readShakapackerConfig has a fallback from shakapacker/package/config → require("shakapacker").config, but no test exercises the path where only the subpath fails while the root shakapacker module succeeds. The rspack test file has an equivalent at line 190 ("falls back to shakapacker.config when the package/config subpath is unresolvable"). Suggest adding a parallel test here:
test("falls back to shakapacker.config when the package/config subpath is unresolvable", () => {
const { appRoot } = createPnpmLikeApp({
configTranspiler: "swc",
transpilers: ["@swc/core", "swc-loader"],
writePackageConfig: false
// shakapacker root module is still written with config: { ... }
})
const result = requireWrapper(appRoot)
// The bundler/transpiler checks should still fire via the fallback.
expect(result.warnings).not.toStrictEqual(
expect.arrayContaining([
expect.objectContaining({ code: "SHAKAPACKER_NO_TRANSPILER" })
])
)
})Without this, a regression that silently breaks the fallback (e.g. require("shakapacker")?.config → require("shakapacker").config typo) would go undetected.
| "shakapacker": "~10.1.0", | ||
| "webpack": "~5.106.2", | ||
| "webpack-cli": "~7.0.2", | ||
| "webpack-assets-manifest": "~6.5.1" | ||
| }, |
There was a problem hiding this comment.
Tight ~ pinning on bundler direct deps may cause nested installs. ~5.106.2 resolves only >=5.106.2 <5.107.0. A user who already has webpack@5.107.x in their own package.json (common — webpack ships patch releases frequently) and adds shakapacker-webpack without first removing their webpack entry will end up with two concurrent webpack installs. Webpack plugins that do identity checks (plugin.webpack === require("webpack")) will break in this state.
The migration guide documents removing the old dep, but the install-without-cleanup window is real. ^5.106.2 would preserve the intent (webpack 5, same floor) while letting npm deduplicate when the user's app already satisfies the range:
| "shakapacker": "~10.1.0", | |
| "webpack": "~5.106.2", | |
| "webpack-cli": "~7.0.2", | |
| "webpack-assets-manifest": "~6.5.1" | |
| }, | |
| "shakapacker": "~10.1.0", | |
| "webpack": "^5.106.2", | |
| "webpack-cli": "^7.0.2", | |
| "webpack-assets-manifest": "^6.5.1" |
Same concern applies to @rspack/core / @rspack/cli in shakapacker-rspack/package.json.
| else | ||
| for pkg in shakapacker shakapacker-webpack shakapacker-rspack; do | ||
| if npm view "$pkg" version --registry https://registry.npmjs.org >/dev/null 2>&1; then | ||
| if ! echo "$NPM_ACCESS_JSON" | grep -Eq "\"$pkg\"[[:space:]]*:[[:space:]]*\"read-write\""; then |
There was a problem hiding this comment.
$pkg is interpolated into an extended-regex pattern without escaping. Current package names (shakapacker, shakapacker-webpack, shakapacker-rspack) are safe because they contain no ERE metacharacters, but this is a latent bug if a future package name ever includes ., +, (, etc.
| if ! echo "$NPM_ACCESS_JSON" | grep -Eq "\"$pkg\"[[:space:]]*:[[:space:]]*\"read-write\""; then | |
| if ! echo "$NPM_ACCESS_JSON" | grep -Fq "\"$pkg\""; then |
Or escape the interpolated value explicitly:
| if ! echo "$NPM_ACCESS_JSON" | grep -Eq "\"$pkg\"[[:space:]]*:[[:space:]]*\"read-write\""; then | |
| escaped_pkg="$(printf '%s' "$pkg" | sed 's/[.[\*^$]/\\&/g')" | |
| if ! echo "$NPM_ACCESS_JSON" | grep -Eq "\"$escaped_pkg\"[[:space:]]*:[[:space:]]*\"read-write\""; then |
| const source = readFileSync(path, "utf8") | ||
| // Each wrapper declares its codes as `const NAME = "SHAKAPACKER_..."`; | ||
| // matching the literal lets us assert string-level equality regardless | ||
| // of how the constant is referenced later in `process.emitWarning`. | ||
| return Array.from(source.matchAll(/"(SHAKAPACKER_[A-Z_]+)"/g)) | ||
| .map((match) => match[1]) | ||
| .sort() |
There was a problem hiding this comment.
This regex matches the pattern anywhere in the source — including inside comments and string literals that aren't constant declarations. If someone adds a comment like // does not emit "SHAKAPACKER_NO_TRANSPILER" to the rspack wrapper, the "rspack wrapper does not declare NO_TRANSPILER" assertion would start failing.
Scoping to lines that look like constant assignments would make the extraction more robust:
| const source = readFileSync(path, "utf8") | |
| // Each wrapper declares its codes as `const NAME = "SHAKAPACKER_..."`; | |
| // matching the literal lets us assert string-level equality regardless | |
| // of how the constant is referenced later in `process.emitWarning`. | |
| return Array.from(source.matchAll(/"(SHAKAPACKER_[A-Z_]+)"/g)) | |
| .map((match) => match[1]) | |
| .sort() | |
| const extractCodes = (path) => { | |
| const source = readFileSync(path, "utf8") | |
| // Match only `const NAME = "SHAKAPACKER_..."` declarations, not | |
| // occurrences in comments or arbitrary string literals. | |
| return Array.from(source.matchAll(/^\s*const\s+\w+\s*=\s*"(SHAKAPACKER_[A-Z_]+)"/gm)) | |
| .map((match) => match[1]) | |
| .sort() | |
| } |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 4 total unresolved issues (including 3 from previous reviews).
Bugbot Autofix is ON, but it could not run because on-demand usage is turned off. To enable Bugbot Autofix, turn on on-demand usage and set a spend limit in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 288ecd1. Configure here.
…es tests Address review feedback on PR #1096: - rakelib/release.rake + scripts/publish-packages.sh: `npm version` only rewrites the `version` field, so a 10.1.0 → 10.2.0 release would publish supplementals still declaring `"shakapacker": "~10.1.0"` — which resolves to >=10.1.0 <10.2.0 and cannot install the new core. The rake task now rewrites `dependencies.shakapacker` to `~<new version>` after each `npm version`, and publish-packages.sh asserts that constraint as a defense-in-depth lockstep check. - scripts/publish-packages.sh: compare Ruby gem version against npm version in npm syntax (via VersionSyntaxConverter) instead of raw string compare, which would false-warn on every prerelease release (10.1.0.beta.1 vs 10.1.0-beta.1). - test/packages/shakapacker-webpack/index.test.js: add the missing fallback-path test that mirrors the rspack suite — exercises `require("shakapacker").config` working when the `shakapacker/package/config` subpath is unresolvable. - test/packages/warning-codes.test.js: scope the SHAKAPACKER_* extraction regex to `const`/`let` declarations so a future comment containing one of the literal codes doesn't break the assertions; add a byte-for-byte parity check for the duplicated `readShakapackerConfig` helper to catch silent drift between the two wrappers. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Thanks for the thorough review. Addressed in a2060c6. Item 1 — Missing fallback test in webpack wrapper ✅ Item 3 — Item 5 — Item 2 — Loose vs tight pinning on bundler direct deps ⏭ Holding tight pins. Item 4 — Unescaped While I was in there, I also fixed two adjacent issues:
|
Code Review — RFC: Tighten dependencies for Shakapacker v11 (#1096)OverviewThis PR ships two new supplemental npm packages ( 🔴 Version mismatch: core
|
| else | ||
| for pkg in shakapacker shakapacker-webpack shakapacker-rspack; do | ||
| if npm view "$pkg" version --registry https://registry.npmjs.org >/dev/null 2>&1; then | ||
| if ! echo "$NPM_ACCESS_JSON" | grep -Eq "\"$pkg\"[[:space:]]*:[[:space:]]*\"read-write\""; then |
There was a problem hiding this comment.
$pkg is interpolated directly into the regex. All current values (shakapacker, shakapacker-webpack, shakapacker-rspack) are safe, but if the loop is ever extended to include a scoped package like @shakacode/foo, the @ would be a regex metacharacter and silently match any character rather than a literal @. A small guard makes this future-proof:
| if ! echo "$NPM_ACCESS_JSON" | grep -Eq "\"$pkg\"[[:space:]]*:[[:space:]]*\"read-write\""; then | |
| if ! echo "$NPM_ACCESS_JSON" | grep -Fq "\"$pkg\""; then |
Using -F (fixed string) eliminates the metacharacter risk. The surrounding whitespace/colon matching in the original pattern isn't strictly necessary since npm's JSON output is consistent, but if you need it, escape $pkg before interpolating.
| # underlying error itself; a follow-up run is safe because the next | ||
| # `is_published` check skips anything that did publish. | ||
| npm view "$1@$2" version --registry https://registry.npmjs.org >/dev/null 2>&1 | ||
| } |
There was a problem hiding this comment.
The comment correctly documents that a transient npm view failure (registry unreachable) is indistinguishable from "not yet published." In that case is_published returns false, the script proceeds to npm publish, and if the package was already published it gets a 403 — same outcome as the partial-publish recovery path. Worth noting in the script's usage header that a 403 on npm publish during recovery is expected and safe to re-run, so the maintainer doesn't lose time investigating the error.
| } catch { | ||
| return false | ||
| } | ||
| } |
There was a problem hiding this comment.
The process.cwd() resolution path is well-documented in the comment above, but one additional edge case worth calling out: if a webpack config file does a chdir before require("shakapacker-webpack") (rare but possible in monorepos or custom bootstrap scripts), both the BUNDLER_MISMATCH and NO_TRANSPILER checks can false-fire or false-pass. Since require is cached after the first successful call, the warnings won't re-fire on subsequent requires — so the window is only the first require in the process. Low risk but potentially surprising in monorepo setups.
| const webpackHelper = extractReadConfigHelper(webpackWrapperPath) | ||
| const rspackHelper = extractReadConfigHelper(rspackWrapperPath) | ||
|
|
||
| expect(webpackHelper).toBe(rspackHelper) |
There was a problem hiding this comment.
The byte-for-byte identity check is clever and catches all drift including comment edits. One practical gotcha: if a developer runs prettier --write on just one of the two wrapper files (e.g. as part of a targeted lint-fix), this test will fail even though there's no functional change. Worth a note in the test description or in the comments above each wrapper's readShakapackerConfig that the block must be kept identical including formatting. Alternatively, extract-then-normalize (strip whitespace before comparing) would make the test format-agnostic while still catching logic drift.
Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 4 total unresolved issues (including 3 from previous reviews).
Bugbot Autofix is ON, but it could not run because the branch was deleted or merged before autofix could start.
Reviewed by Cursor Bugbot for commit e89ad18. Configure here.
| : null | ||
| const hasExpectedTranspiler = expectedGroup | ||
| ? expectedGroup.every(canResolve) | ||
| : Object.values(transpilerGroups).some((group) => group.every(canResolve)) |
There was a problem hiding this comment.
Transpiler group lookup is case-sensitive despite case-insensitive none check
Low Severity
The none check uses transpilerSetting?.toLowerCase() for case-insensitive comparison, but the transpilerGroups lookup on the next line uses transpilerSetting without lowercasing. When a value like "SWC" arrives via the SHAKAPACKER_JAVASCRIPT_TRANSPILER env var (which config.ts passes through without normalizing case), transpilerGroups["SWC"] returns undefined, causing expectedGroup to be falsy and falling back to the weaker "any pair resolves" check instead of the specific SWC pair check. A user with SHAKAPACKER_JAVASCRIPT_TRANSPILER=SWC who has babel (but not swc) installed would get no warning and then hit a cryptic loader error at build time.
Reviewed by Cursor Bugbot for commit e89ad18. Configure here.
|
Test review comment |
|
Code Review - PR 1096: Tighten Dependencies for Shakapacker v11 Overview This PR introduces a phased dependency-tightening strategy via two new optional supplemental npm packages. The design is well-considered: separate packages cleanly solve the conditional peer dep problem that a single package.json cannot express. The release automation, runtime warnings, and test coverage are thorough. What works well
Issues Minor:
Design note (no action required, but worth documenting in the migration guide):
|
| @@ -0,0 +1,20 @@ | |||
| Copyright (c) 2026 Justin Gordon, ShakaCode | |||
There was a problem hiding this comment.
The rspack LICENSE uses Copyright (c) 2016-2026 but this one says 2026 only. Shakapacker dates to 2016, so both should use the same range.
| Copyright (c) 2026 Justin Gordon, ShakaCode | |
| Copyright (c) 2016-2026 Justin Gordon, ShakaCode |
| // don't know what the user configured. Defaulting to a specific pair | ||
| // would produce false positives — e.g., the install template ships | ||
| // `javascript_transpiler: "swc"`, so a user with that standard install | ||
| // plus a transient config load failure has SWC installed, not Babel. | ||
| const expectedGroup = transpilerSetting |
There was a problem hiding this comment.
The none check above (line 115) normalizes case with .toLowerCase(), but the transpilerGroups lookup here is case-sensitive. If a user writes javascript_transpiler: "SWC" (uppercase), transpilerGroups["SWC"] returns undefined and falls through to the "any installed pair" fallback — they'd get a generic warning or no warning at all instead of the specific "@swc/core + swc-loader not installed" diagnostic.
Consider normalizing before the lookup for consistency:
| // don't know what the user configured. Defaulting to a specific pair | |
| // would produce false positives — e.g., the install template ships | |
| // `javascript_transpiler: "swc"`, so a user with that standard install | |
| // plus a transient config load failure has SWC installed, not Babel. | |
| const expectedGroup = transpilerSetting | |
| const normalizedTranspiler = transpilerSetting?.toLowerCase() | |
| const expectedGroup = normalizedTranspiler | |
| ? transpilerGroups[normalizedTranspiler] | |
| : null |
(And update the transpilerSetting references inside the if (!hasExpectedTranspiler) block to use normalizedTranspiler when building the warning message.)
| const effectiveBundler = bundlerSetting ?? "webpack" | ||
| if (shakapackerConfig !== undefined && effectiveBundler !== "rspack") { |
There was a problem hiding this comment.
When assets_bundler is not set in shakapacker.yml at all (a valid rspack setup prior to this PR — rspack required the explicit setting but many apps relied on @rspack/core being present), bundlerSetting is undefined, effectiveBundler becomes "webpack", and the warning fires.
This is intentional behavior (documented in the PR), but it means any app that adopted rspack without explicitly writing assets_bundler: rspack will see a new SHAKAPACKER_BUNDLER_MISMATCH warning after installing shakapacker-rspack, even though their build is still working correctly. Worth adding a one-liner to the migration guide so adopters are not caught off guard by a warning that disappears as soon as they add one config line.
## Summary Addresses [#1131](#1131) (G-Rath) on the v10.1.0-rc.1 supplemental packages. Three structural changes: 1. **Bundler singletons → required peer dependencies.** `shakapacker-webpack` moves `webpack`, `webpack-cli`, and `webpack-assets-manifest` from `dependencies` to required `peerDependencies`. `shakapacker-rspack` does the same for `@rspack/core`, `@rspack/cli`, and `rspack-manifest-plugin`. Eliminates the silent duplicate-bundler failure mode where an app or transitive dep pinning a different bundler version could end up with two webpacks in the tree (and silently broken `instanceof` checks in plugins/loaders). 2. **`~` → `^` for all non-`shakapacker` deps.** Tilde locked the supplemental to the upstream patch range, forcing a coordinated release for every webpack/rspack/loader minor. Caret lets users pick up compatible upstream releases without waiting. `shakapacker` itself stays tilde-pinned because the wrapper imports core's internal `package/config` subpath and needs the matching minor. 3. **`terser-webpack-plugin` → direct dependency of `shakapacker-webpack`.** `package/optimization/webpack.ts` does `requireOrError("terser-webpack-plugin")` for the default minimizer, so it's always required for any production build using shakapacker's defaults. Moves from optional peer (where npm doesn't warn when missing) to direct dep. Plus one bonus cleanup raised in the issue thread: the supplemental peer ranges were *tighter* than main `shakapacker`'s peer ranges (e.g., `webpack-dev-server: ~5.2.3` vs main's `^5.2.2`). Widened them to match — the supplemental should never narrow what bare core accepts. Review confirmation: `@rspack/plugin-react-refresh` now intentionally uses `^1.0.0 || ^2.0.0` in both core and the rspack supplemental. Rspack 2.x is stable, so this excludes `2.0.0-*` prereleases instead of preserving the older `^2.0.0-0` prerelease-compatible range. ## Behavior on each package manager | Package manager | Behavior after this change | | -------------------------------------------- | ------------------------------------------------------------------------------------- | | npm 7+ | Auto-installs required peers; wrapper-only install works | | pnpm | Strict app-level imports still need explicit deps; see migration guide | | yarn 2+ (Berry) — PnP mode | Strict app-level imports still need explicit deps; see migration guide | | yarn 2+ (Berry) — `nodeLinker: node-modules` | Hoisted node_modules mode behaves like npm 7+ | | yarn 1 (classic) | Surfaces peer warnings listing missing packages | The Rails `shakapacker:install` task continues to write all required deps explicitly into the user's `package.json` (it doesn't use the supplemental packages), so installed apps still get true one-command UX on every package manager. ## What this PR doesn't do - **No installer code changes.** The Rails installer (`lib/install/template.rb`) already provides true one-command UX by writing core + all peers explicitly. Switching the installer default to use supplemental packages is a behavior change beyond #1131's scope — PR #1096 explicitly kept supplemental adoption opt-in for the v10.x line. - **No changes to main `shakapacker`'s `package.json`.** It already follows these conventions (webpack as a peer with `^5.101.0`, terser-webpack-plugin as a peer with `^5.3.1`). - **No v11 changes.** This PR only touches v10.1 RC shape so the GA ships with the right dependency topology. The v11 roadmap (remove bundler peers from core) remains gated on bundler-specific runtime imports being fully owned by the supplemental packages — see `docs/dependency-strategy.md`. ## Files changed - `packages/shakapacker-webpack/package.json` — restructure per recommendations - `packages/shakapacker-rspack/package.json` — restructure per recommendations - `test/packages/package-metadata.test.js` (new) — locks in the contract (singletons are required peers, terser-webpack-plugin is direct dep, no `~` outside `shakapacker`, supplemental peer ranges don't narrow main's) - `docs/dependency-strategy.md` — rewrite summary, per-package tables, Version Pinning Philosophy, examples, Resolved Decisions - `docs/migration/v10.1-supplemental-packages.md` — add PM support table, split "After" examples by PM - `docs/blog/2026-05-10-shakapacker-10-1-supplemental-packages.md` — revise 30-second pitch and What Changed - `packages/shakapacker-{webpack,rspack}/README.md` — rewrite Install sections with auto-peer-install vs yarn-1 commands - `CHANGELOG.md` — Unreleased entry documenting the change ## Test plan - [x] `yarn jest test/packages/package-metadata.test.js` — 12 new tests pass (asserts the contract above) - [x] `yarn jest` — full JS suite: 514 tests, 53 suites, all pass - [x] `yarn lint` — clean - [x] `yarn prettier --check` — clean (after autofix) - [ ] Manual install smoke tests on npm 7+, pnpm, yarn 1, yarn 2+ — to be run on the published rc.2 tarball before promoting to GA Fixes #1131 <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Medium risk because it changes published npm dependency topology for `shakapacker-webpack`/`shakapacker-rspack`, which can affect install/resolution behavior across package managers (npm vs pnpm/Yarn PnP) and production builds relying on `terser-webpack-plugin`. > > **Overview** > **Restructures the supplemental npm packages’ dependency model** so bundler singletons are no longer bundled as direct deps. > > `shakapacker-webpack` now declares `webpack`, `webpack-cli`, and `webpack-assets-manifest` as **required `peerDependencies`** (and moves `terser-webpack-plugin` to a direct `dependency`), while `shakapacker-rspack` similarly peers `@rspack/core`, `@rspack/cli`, and `rspack-manifest-plugin`. Version constraints are loosened from `~` to `^`/broader ranges for non-`shakapacker` packages (including `esbuild`), and docs/migration guidance is updated to spell out npm 7+ auto-peer-install vs pnpm/Yarn PnP explicit-deps requirements. > > Adds new Jest coverage (`test/packages/package-metadata.test.js` and an opt-in `install-smoke.test.js`) to lock in the peer/dependency contract and validate resolution behavior across npm/pnpm, plus updates the changelog/blog/design-rationale docs accordingly. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit dc1c725. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Chores** * Supplemental bundler packages now declare the bundler as required peer dependencies; `terser-webpack-plugin` is a direct dependency for the webpack wrapper. Version ranges loosened (tilde → caret) for broader compatibility. * **Documentation** * Installation, migration, and blog guidance updated with npm/pnpm/Yarn PnP install behavior and one-command npm 7+ flows. * **Tests** * Added tests validating package metadata and install-resolution behaviors across npm and pnpm. <!-- review_stack_entry_start --> [](https://app.coderabbit.ai/change-stack/shakacode/shakapacker/pull/1133?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack) <!-- review_stack_entry_end --> <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
](https://app.coderabbit.ai/change-stack/shakacode/shakapacker/pull/1133?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack){kind=link}
## Summary Stamps the v10.1.0 stable release header (was `v10.1.0-rc.2`) and curates the prerelease entries per `/update-changelog` guidance. ## Curation applied - **Merged PR #1133 → PR #1096** (Added: supplemental npm packages). #1133 restructured the supplemental deps from direct → peer in rc.2, so the original entry's "bundle the managed-build stack as direct dependencies" description was wrong for stable consumers. Rewrote to describe the final peer-deps shape (auto-installed on npm 7+) and credit both PRs. The #1133 standalone Changed entry was removed. - **Merged PR #1132 → PR #1104** (Fixed: ESM binstubs). #1132 fixed the `export_bundler_config` dispatch regression that #1104 introduced in rc.1 when switching from JS to Ruby binstubs; stable users never see the bug, so the fix is RC-only churn that belongs folded into the original entry. Also folded in #1132's minor improvements (NODE_ENV from RAILS_ENV mapping, Node lookup without execution, friendlier non-Error CLI messages). - **Merged PR #1106 → PR #1086** (Fixed: local-path version detection). Both shipped in rc.0 as iterations of the same regex fix; merged into a single entry covering `../`, `./`, and `file:` patterns. - **Lightened Migration Notes**: removed "bundled stack" language now that supplementals declare required peers instead of direct deps. - **Skipped post-rc.2 commit**: PR #1138 (skill tooling guidance) is internal — not user-visible. - **Compare links**: `[Unreleased]` now points from `v10.1.0`; replaced `[v10.1.0-rc.2]` with `[v10.1.0]: v10.0.0...v10.1.0`. ## Test plan - [x] `yarn lint` passes - [x] `yarn prettier --check CHANGELOG.md` passes - [x] File ends with trailing newline - [x] No duplicate section headings - [x] All merged PRs credited After merge, run `bundle exec rake release` (no args) — the rake task picks up `v10.1.0` from CHANGELOG.md and auto-creates the GitHub release. 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > Documentation-only changelog edits with no runtime or dependency behavior changes. > > **Overview** > **Curates `CHANGELOG.md` for the v10.1.0 stable release** (replacing the `v10.1.0-rc.2` header dated May 25, 2026) so release notes match what stable users actually ship. > > Prerelease churn is folded into fewer, accurate bullets: supplemental npm packages (**#1096** + **#1133**) now describe **required peer dependencies** (not direct “bundled” deps), with the standalone **#1133** *Changed* entry removed; ESM/config binstub fixes (**#1104** + **#1132**) and local-path version detection (**#1086** + **#1106**) are single *Fixed* items. **Migration Notes** no longer imply a direct bundled stack on supplementals. > > Footer compare links: **`[Unreleased]`** from `v10.1.0`, **`[v10.1.0]`** replaces the rc.2 link (`v10.0.0...v10.1.0`). > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 2b57d5f. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 participant
Summary
This PR ships Phase 1 of the v11 dependency-tightening plan: the supplemental packages and the migration story for v10.1.
What's in this PR
packages/shakapacker-webpackandpackages/shakapacker-rspack— thin wrappers around core that bundle the managed-build stack as directdependencies(so a singleyarn add shakapacker-webpackpullsshakapacker,webpack,webpack-cli, andwebpack-assets-manifest;shakapacker-rspackdoes the same for@rspack/core,@rspack/cli, andrspack-manifest-plugin). Optional features (transpilers, dev-server, CSS preprocessors, react-refresh) stay as opt-inpeerDependenciesso SCSS/native-binding bloat isn't forced on every install. Adoption is opt-in; existing apps onshakapackerkeep working.SHAKAPACKER_BUNDLER_MISMATCH,SHAKAPACKER_NO_TRANSPILER) emitted viaprocess.emitWarningso users see actionable preflight messages before webpack/rspack throw cryptic loader errors. Suppressible with--no-warnings../package/configsubpath export so the wrappers can readconfig/shakapacker.ymlwithout transitively loading bundler-specific modules. (The wildcard./package/*doesn't append.js, so the explicit entry is required for production installs.)scripts/publish-packages.shorchestrator: publishes core then supplementals in lockstep with idempotency guards, SLSA provenance, pre-release tag enforcement, and version-match checks.10.1.0; refreshed dev dependency pins to the tested stack (webpack/webpack-cli/webpack-assets-manifest, Rspack v2, SWC/Babel/esbuild loaders).Documentation
docs/migration/v10.1-supplemental-packages.md— user-facing migration guide with before/afterpackage.jsonsnippets for rspack, webpack, and custom-build apps.docs/blog/2026-05-10-shakapacker-10-1-supplemental-packages.md— release announcement post covering the simpler install, runtime safety net, and v11 outlook.docs/dependency-strategy.md— design rationale, alternatives considered (Vite/Next.js comparison), version pinning philosophy, monorepo + lockstep release strategy, and v11 roadmap. Renamed fromdocs/rfcs/0001-tighten-dependencies-v11.mdsince it's no longer an RFC.What's not in this PR (planned for v11)
The v11 phase is the working roadmap, not a scheduled release. The supplemental packages are intentionally opt-in for the entire 10.x line so v10.1 can soak before any breaking changes land. When v11 ships it will:
shakapacker)See
docs/dependency-strategy.mdfor the full rationale and Phase 2 sketch.Related follow-ups
Test plan
yarn jest test/packages/shakapacker-webpack/index.test.js— 15 wrapper tests covering transpiler-pair detection, bundler mismatch, config load failure paths, double-warning suppressionyarn jest test/packages/shakapacker-rspack/index.test.js— wrapper tests for the rspack supplementalyarn jest test/packages/subpath-exports.test.js— verifiesrequire("shakapacker/package/config")resolves in the published layout, not just the test sandboxyarn jest test/packages/warning-codes.test.js— regression test pinning the warning code constants so the two wrappers can't driftscripts/publish-packages.shagainst a local registry before first real publish🤖 Generated with Claude Code
Note
Medium Risk
Introduces new publishable npm packages plus release/publish automation and new runtime warning preflights, which could impact install/release flows and module resolution for consumers adopting the wrappers.
Overview
Adds two optional, lockstep npm wrappers (
shakapacker-webpack,shakapacker-rspack) that bundle the managed bundler stacks as direct dependencies and keep optional features as opt-in peer deps.The wrappers preflight configuration and emit structured warnings (
SHAKAPACKER_BUNDLER_MISMATCH,SHAKAPACKER_NO_TRANSPILER) and core now explicitly exportsshakapacker/package/configvia./package/configfor lightweight config reads.Updates release automation to bump/publish all three npm packages in order (core first) via a new
scripts/publish-packages.sh, and refreshes dev dependency pins; adds migration/design docs and a 10.1 announcement blog post documenting adoption and the v11 roadmap.Reviewed by Cursor Bugbot for commit e89ad18. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit
New Features
Documentation
Chores