Move build tooling packages from devDependencies to dependencies#929
Move build tooling packages from devDependencies to dependencies#929
Conversation
WalkthroughThis PR reclassifies 16 build tooling packages from Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 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 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 ReviewThis PR moves build tooling from Core Issue: This Approach is Architecturally Incorrect for a LibraryShakapacker is a library published to npm. For libraries, the npm dependency sections have specific semantics:
The packages moved here (webpack, rspack, loaders, plugins) should remain as peer dependencies only. They're tools the consumer orchestrates — shakapacker doesn't import them at runtime, it just knows how to configure them. Problem 1: Duplication Between
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| "prepublishOnly": "yarn build && yarn type-check" | ||
| }, | ||
| "dependencies": { | ||
| "@rspack/cli": "^1.5.8", |
There was a problem hiding this comment.
These packages are already declared in peerDependencies (lines 102–123). Having them in both dependencies and peerDependencies will cause package managers to install duplicate copies — one pinned by shakapacker's dependency resolution, and one from the consumer's own install. This is a common source of version conflicts (e.g., two webpack instances) and will silently inflate every consumer's node_modules.
| "rspack-manifest-plugin": "^5.0.3", | ||
| "sass-loader": "^16.0.5", | ||
| "swc-loader": "^0.1.15", | ||
| "webpack": "5.93.0", |
There was a problem hiding this comment.
This exact pin (5.93.0, no caret) conflicts with the peerDependencies range ^5.76.0. A consumer on webpack 5.94.0 will end up with two different webpack versions installed simultaneously — a well-known cause of bundler bugs (duplicate plugin instances, HMR breakage, etc.). If kept here at all, this should be "webpack": "^5.76.0" to match the peer dep range.
Greptile SummaryMoves essential build tooling packages (webpack, rspack, loaders, and plugins) from
Confidence Score: 5/5
Important Files Changed
Last reviewed commit: 09a9f23 |
| "mini-css-extract-plugin": "^2.9.4", | ||
| "path-complete-extname": "^1.0.0", | ||
| "rspack-manifest-plugin": "^5.0.3", | ||
| "sass-loader": "^16.0.5", |
There was a problem hiding this comment.
sass-loader is now a hard dependency, but sass itself remains only in peerDependencies (optional). This is inconsistent — sass-loader is non-functional without sass. Consumers who don't use Sass now get sass-loader forced on them but don't get sass, so the loader serves no purpose in their install.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Bugbot Autofix is ON, but it could not run because Privacy Mode (Legacy) is turned on. To enable Bugbot Autofix, switch your privacy mode in the Cursor dashboard.
| "rspack-manifest-plugin": "^5.0.3", | ||
| "sass-loader": "^16.0.5", | ||
| "swc-loader": "^0.1.15", | ||
| "webpack": "5.93.0", |
There was a problem hiding this comment.
Exact-pinned webpack version forces duplicates on consumers
High Severity
webpack is exact-pinned at "5.93.0" (no caret) in dependencies, while the peerDependencies declares "^5.76.0". When this was in devDependencies, the exact pin only affected local development. Now that it's a hard dependency, consumers using any other webpack 5.x version (e.g., 5.95.0) will get a duplicate webpack installation nested inside this package's node_modules. Two webpack instances cause instanceof check failures, broken singleton patterns, and plugin incompatibilities. Every other package in dependencies uses a caret range — this one likely needs ^5.93.0 or similar.
| "webpack-assets-manifest": "^5.0.6", | ||
| "webpack-cli": "^6.0.0", | ||
| "webpack-merge": "^5.8.0", | ||
| "webpack-subresource-integrity": "^5.1.0", |
There was a problem hiding this comment.
Mutually exclusive optional packages now forced on all consumers
Medium Severity
Packages like @rspack/cli, @rspack/core, webpack, webpack-cli, babel-loader, swc-loader, and esbuild-loader are designed to be mutually exclusive choices (webpack vs rspack, babel vs swc vs esbuild). They were previously optional peer dependencies so consumers install only what they need. Moving all of them into hard dependencies forces every consumer to install both bundlers and all transpiler loaders, significantly bloating installs. The peerDependenciesMeta optional: true flags are now contradicted and rendered meaningless.
![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
🧹 Nitpick comments (1)
package.json (1)
52-52:esbuild-loaderis pinned two major versions behind its current release
"esbuild-loader": "^2.18.0"pins to the v2 line. The latest published version ofesbuild-loaderis 4.4.2. ThepeerDependenciesalready advertises support up to^4.0.0. Keepingdependencieson^2means every consumer gets v2 regardless of what newer version they have installed, and they miss 2 major versions of improvements.Consider bumping to
"^4.0.0"(or at least"^3.0.0 || ^4.0.0") to stay aligned with the latest supported range.🔧 Proposed fix
- "esbuild-loader": "^2.18.0", + "esbuild-loader": "^4.0.0",Similarly,
babel-loaderindependenciesis pinned to^8.2.4. The latest version ofbabel-loaderis 10.0.0. ThepeerDependenciesalready allows^10.0.0.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@package.json` at line 52, Update package.json dependencies to allow current major versions: replace the esbuild-loader entry ("esbuild-loader": "^2.18.0") with a range that matches supported peers, e.g. "^4.0.0" (or "^3.0.0 || ^4.0.0") so consumers can resolve modern v3/ v4 releases; likewise bump babel-loader from "^8.2.4" to "^10.0.0" to match its peer support. Ensure only the dependency version strings for "esbuild-loader" and "babel-loader" are updated in package.json.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@package.json`:
- Line 59: The package.json dependency for "webpack" is pinned to an exact patch
("5.93.0") which forces consumers to install duplicate webpack instances; update
the "webpack" entry in package.json to use a caret range (e.g. change "webpack":
"5.93.0" to "webpack": "^5.93.0") so npm/yarn can resolve compatible versions
and avoid duplicate installs (ensure this aligns with the existing
peerDependencies range if needed).
- Around line 49-63: package.json lists specific dependency ranges narrower than
the package's peerDependencies which can cause duplicate installs and lock
consumers to older majors; update the dependency ranges for babel-loader,
esbuild-loader, compression-webpack-plugin, swc-loader, and
webpack-assets-manifest to either match the widest peerDependencies ranges (e.g.
"babel-loader": "^8.2.4 || ^9.0.0 || ^10.0.0", "esbuild-loader": "^2.0.0 ||
^3.0.0 || ^4.0.0", etc.) or bump each dependency to the latest major allowed by
its peerDependencies (e.g. set babel-loader to "^10.0.0") so npm dedupes
correctly and avoids installing a second copy.
---
Nitpick comments:
In `@package.json`:
- Line 52: Update package.json dependencies to allow current major versions:
replace the esbuild-loader entry ("esbuild-loader": "^2.18.0") with a range that
matches supported peers, e.g. "^4.0.0" (or "^3.0.0 || ^4.0.0") so consumers can
resolve modern v3/ v4 releases; likewise bump babel-loader from "^8.2.4" to
"^10.0.0" to match its peer support. Ensure only the dependency version strings
for "esbuild-loader" and "babel-loader" are updated in package.json.
| "babel-loader": "^8.2.4", | ||
| "compression-webpack-plugin": "^9.0.0", | ||
| "css-loader": "^7.1.2", | ||
| "esbuild-loader": "^2.18.0", | ||
| "js-yaml": "^4.1.0", | ||
| "mini-css-extract-plugin": "^2.9.4", | ||
| "path-complete-extname": "^1.0.0", | ||
| "rspack-manifest-plugin": "^5.0.3", | ||
| "sass-loader": "^16.0.5", | ||
| "swc-loader": "^0.1.15", | ||
| "webpack": "5.93.0", | ||
| "webpack-assets-manifest": "^5.0.6", | ||
| "webpack-cli": "^6.0.0", | ||
| "webpack-merge": "^5.8.0", | ||
| "webpack-subresource-integrity": "^5.1.0", |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat package.jsonRepository: shakacode/shakapacker
Length of output: 6578
🏁 Script executed:
tail -c 1 package.json | od -An -tx1Repository: shakacode/shakapacker
Length of output: 107
🏁 Script executed:
tail -c 1 package.json | wc -cRepository: shakacode/shakapacker
Length of output: 66
🏁 Script executed:
tail -c 2 package.json | xxdRepository: shakacode/shakapacker
Length of output: 108
🏁 Script executed:
python3 << 'EOF'
with open('package.json', 'rb') as f:
content = f.read()
last_char = content[-1:]
if last_char == b'\n':
print("File ends with newline")
else:
print(f"File does NOT end with newline. Last byte: {last_char}")
EOFRepository: shakacode/shakapacker
Length of output: 87
Several dependencies ranges are narrower than their peerDependencies counterparts, risking duplicate installs and forcing consumers to stale majors
When Shakapacker is a dependency of a consuming project, npm 7+ resolves Shakapacker's own dependencies separately. If the consumer has a version that satisfies the peerDependencies range but not the dependencies range, npm installs a second copy of the conflicting package inside Shakapacker's subtree. This results in:
- Two copies of the same loader/plugin in the build (known to cause subtle runtime failures e.g. with webpack plugin registrations)
- Consumers being silently locked to an older major despite their
peerDependenciesmeta advertising broader support
Affected pairs (dependencies → peerDependencies):
| Package | dependencies |
peerDependencies |
|---|---|---|
babel-loader |
^8.2.4 |
^8.2.4 || ^9.0.0 || ^10.0.0 |
esbuild-loader |
^2.18.0 |
^2.0.0 || ^3.0.0 || ^4.0.0 |
compression-webpack-plugin |
^9.0.0 |
^9.0.0 || ^10.0.0 || ^11.0.0 |
swc-loader |
^0.1.15 |
^0.1.15 || ^0.2.0 |
webpack-assets-manifest |
^5.0.6 |
^5.0.6 || ^6.0.0 |
The dependencies range should either:
- Match the widest range declared in
peerDependencies(e.g."^8.2.4 || ^9.0.0 || ^10.0.0"), so deduplication succeeds for any consumer-compatible version, or - Be bumped to the latest major that
peerDependenciessupports (e.g."^10.0.0"for babel-loader).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@package.json` around lines 49 - 63, package.json lists specific dependency
ranges narrower than the package's peerDependencies which can cause duplicate
installs and lock consumers to older majors; update the dependency ranges for
babel-loader, esbuild-loader, compression-webpack-plugin, swc-loader, and
webpack-assets-manifest to either match the widest peerDependencies ranges (e.g.
"babel-loader": "^8.2.4 || ^9.0.0 || ^10.0.0", "esbuild-loader": "^2.0.0 ||
^3.0.0 || ^4.0.0", etc.) or bump each dependency to the latest major allowed by
its peerDependencies (e.g. set babel-loader to "^10.0.0") so npm dedupes
correctly and avoids installing a second copy.
| "rspack-manifest-plugin": "^5.0.3", | ||
| "sass-loader": "^16.0.5", | ||
| "swc-loader": "^0.1.15", | ||
| "webpack": "5.93.0", |
There was a problem hiding this comment.
webpack is pinned to an exact version — use a caret range instead
"webpack": "5.93.0" (no ^) locks every consumer to exactly this patch. When a consuming project has a different webpack version (even a patch bump like 5.94.0), npm/yarn will install a second, nested copy of webpack. Two concurrent webpack instances in one build cause hard-to-debug loader resolution and plugin registration failures. The intent is clearly a range — peerDependencies already declares "^5.76.0".
🔧 Proposed fix
- "webpack": "5.93.0",
+ "webpack": "^5.93.0",📝 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.
| "webpack": "5.93.0", | |
| "webpack": "^5.93.0", |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@package.json` at line 59, The package.json dependency for "webpack" is pinned
to an exact patch ("5.93.0") which forces consumers to install duplicate webpack
instances; update the "webpack" entry in package.json to use a caret range (e.g.
change "webpack": "5.93.0" to "webpack": "^5.93.0") so npm/yarn can resolve
compatible versions and avoid duplicate installs (ensure this aligns with the
existing peerDependencies range if needed).
1 participant
Summary
devDependenciestodependenciesdevDependenciesWhy
Issue #662 reports production install/build failures when package managers skip
devDependencies. Moving these runtime bundler packages intodependenciesensures they are available in production installs and file-path based local installs.Fixes #662
Note
Low Risk
Dependency classification-only change with no runtime code modifications; main risk is downstream install size/duplicate dependency expectations.
Overview
Moves core bundler/build tooling packages (Rspack/Webpack, loaders, and related plugins) from
devDependenciesintodependenciesso production installs that omit dev deps still have the build toolchain available.Keeps the lint/test/typecheck toolchain in
devDependenciesand leaves existingpeerDependencies/peerDependenciesMetadeclarations unchanged.Written by Cursor Bugbot for commit 09a9f23. Configure here.
Summary by CodeRabbit