chore(deps): audit deps, move several to devDependencies#524
chore(deps): audit deps, move several to devDependencies#524
Conversation
Three categories of changes: - A few deps were unused and have been removed - Some webpack loaders are used only during build and have moved to `devDependencies` - `scratch-render` exposes two loaders to library consumers when those consumers use the `webpack` export specifically. Those loaders were listed as `dependencies`, but it's more correct to list them as `devDependencies` (for `scratch-render` build) as well as optional `peerDependencies` (for webpack consumers)
There was a problem hiding this comment.
Pull request overview
This PR audits dependency declarations across the monorepo packages, removing unused packages and reclassifying build-time webpack loaders so that runtime consumers aren’t forced to install them.
Changes:
- Remove unused loader deps from
scratch-vmandscratch-svg-renderer. - Move build-only webpack loaders (and related tooling deps) in
scratch-gui/scratch-renderintodevDependencies. - For
scratch-render, re-declareify-loader/raw-loaderas optional peer dependencies for consumers using thewebpackexport condition, and updatepackage-lock.jsonaccordingly.
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/scratch-vm/package.json | Drops arraybuffer-loader from runtime dependencies. |
| packages/scratch-svg-renderer/package.json | Drops base64-loader from runtime dependencies. |
| packages/scratch-render/package.json | Moves ify-loader/raw-loader to devDeps and adds them as optional peer deps for webpack consumers. |
| packages/scratch-gui/package.json | Moves various build/test tooling and webpack loaders to devDependencies; removes react-redux from dependencies. |
| package-lock.json | Re-locks dependency graph to reflect dependency/devDependency/peerDependency changes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "react-ga": "3.3.1", | ||
| "react-intl": "6.8.9", | ||
| "react-modal": "3.16.3", | ||
| "react-popover": "0.5.10", |
There was a problem hiding this comment.
react-redux is still imported throughout scratch-gui (e.g. many connect/Provider usages), but it has been removed from dependencies and is now only declared as a peerDependency. This is a behavioral change for consumers whose package manager does not auto-install peer deps (or has peer auto-install disabled): they will now get runtime/build failures unless they add react-redux explicitly. If this change is intentional, it should be called out as a breaking change in release notes; otherwise consider keeping react-redux as a direct dependency (while also keeping it as a peer to avoid duplicate React trees).
| "react-popover": "0.5.10", | |
| "react-popover": "0.5.10", | |
| "react-redux": "9.1.2", |
| }, | ||
| "peerDependenciesMeta": { | ||
| "ify-loader": { | ||
| "optional": true | ||
| }, | ||
| "raw-loader": { | ||
| "optional": true | ||
| } |
There was a problem hiding this comment.
ify-loader and raw-loader are required by the webpack export path (inline !ify-loader! / raw-loader! usage in src/), but they are declared as optional peerDependencies. With this setup, webpack consumers who rely on the webpack condition can hit a build-time "module not found" without any peer-dep warning during install. Consider making these peer deps non-optional (and instead document that they’re only needed when using the webpack export), or add a clearer install-time signal via documentation/tooling so webpack consumers discover the requirement earlier.
| }, | |
| "peerDependenciesMeta": { | |
| "ify-loader": { | |
| "optional": true | |
| }, | |
| "raw-loader": { | |
| "optional": true | |
| } |
Test Results 8 files 1 078 suites 12m 42s ⏱️ Results for commit f51568c. ♻️ This comment has been updated with latest results. |
|
I want to think more carefully about the |
Proposed Changes
Three categories of changes:
devDependenciesscratch-renderexposes two loaders to library consumers when those consumers use thewebpackexport specifically. Those loaders were listed asdependencies, but it's more correct to list them asdevDependencies(forscratch-renderbuild) as well as optionalpeerDependencies(for webpack consumers)Test Coverage
Covered by the existing build & test steps