refactor(plugin-svgr): use internal asset loader#7482
Conversation
There was a problem hiding this comment.
Pull request overview
This PR refactors @rsbuild/plugin-svgr to stop relying on prebundled/patch-managed url-loader + file-loader for mixed SVG imports, and instead uses a new internal SVG asset loader to preserve the same “inline under limit, otherwise emit file” behavior with fewer external build-time hacks.
Changes:
- Replace the
url-loader-based mixed import handling with a new internalassetLoader(built asassetLoader.mjs). - Remove
prebundlesetup and thefile-loader/url-loaderpatched dependency workflow. - Update snapshots and package metadata to reflect the new loader path and dependency set.
Reviewed changes
Copilot reviewed 9 out of 11 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| pnpm-workspace.yaml | Removes patched dependency entries for file-loader / url-loader. |
| pnpm-lock.yaml | Drops file-loader, url-loader, and related transitive deps from the lockfile. |
| patches/url-loader@4.1.1.patch | Deletes the patch since url-loader is no longer used. |
| patches/file-loader@6.2.0.patch | Deletes the patch since file-loader is no longer used. |
| packages/plugin-svgr/tests/snapshots/index.test.ts.snap | Updates snapshot to expect the new internal loader path. |
| packages/plugin-svgr/src/loader-utils.d.ts | Adds local typing for loader-utils.interpolateName usage in the new loader. |
| packages/plugin-svgr/src/index.ts | Switches mixed-import URL handling to assetLoader.mjs. |
| packages/plugin-svgr/src/assetLoader.ts | Introduces the internal raw loader implementing inline-or-emit SVG asset behavior. |
| packages/plugin-svgr/rslib.config.ts | Adds assetLoader as a build entry and outputs it as .mjs. |
| packages/plugin-svgr/prebundle.config.ts | Removes prebundle configuration (no longer needed). |
| packages/plugin-svgr/package.json | Removes file-loader, url-loader, and prebundle from devDependencies and scripts; keeps package files focused on dist. |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
Comments suppressed due to low confidence (1)
packages/plugin-svgr/src/index.ts:263
generatorOptions?.filenamecan be anAssetModuleFilenamefunction (not just a string) and may also be undefined; passing it as thenameoption will breakassetLoader(it relies onloader-utils.interpolateName, which expects a template string or a (resourcePath, resourceQuery) callback). Please ensure the option passed here is always a string template (e.g. fall back to Rsbuild’s default svg filename) and handle the function case explicitly (e.g. by deriving a string template or using a different code path).
.use(CHAIN_ID.USE.URL)
.loader(path.resolve(__dirname, './assetLoader.mjs'))
.options({
limit: maxSize,
name: generatorOptions?.filename,
});
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Code Review
This pull request replaces the external url-loader and file-loader dependencies with a custom internal assetLoader for SVG assets, removing legacy patches and the prebundling process. Feedback identifies a missing loader-utils dependency in package.json, a logic error in assetLoader.ts when handling function-based name templates, and the fragility of hardcoding the .mjs extension in the loader path.
Summary
This PR replaces the prebundled
url-loader/file-loaderpath in@rsbuild/plugin-svgrwith an internal SVG asset loader. It keeps the mixed import asset handling intact while removing the extra package patches and prebundle setup.Related Links
None