-
Notifications
You must be signed in to change notification settings - Fork 111
feat(rspeedy/core): support resolve.dedupe
#1671
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
🦋 Changeset detectedLatest commit: 9f92b44 The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Caution Review failedAn error occurred during the review process. Please try again later. ✨ Finishing Touches🧪 Generate unit tests
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 |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
React Example#4929 Bundle Size — 238.26KiB (0%).9f92b44(current) vs a956600 main#4916(baseline) Bundle metrics
|
| Current #4929 |
Baseline #4916 |
|
|---|---|---|
0B |
0B |
|
0B |
0B |
|
0% |
0% |
|
0 |
0 |
|
4 |
4 |
|
164 |
164 |
|
68 |
68 |
|
46.93% |
46.93% |
|
2 |
2 |
|
0 |
0 |
Bundle size by type no changes
| Current #4929 |
Baseline #4916 |
|
|---|---|---|
145.76KiB |
145.76KiB |
|
92.5KiB |
92.5KiB |
Bundle analysis report Branch colinaaa:colin/0908/dedupe Project dashboard
Generated by RelativeCI Documentation Report issue
Web Explorer#4920 Bundle Size — 367.43KiB (0%).9f92b44(current) vs a956600 main#4907(baseline) Bundle metrics
|
| Current #4920 |
Baseline #4907 |
|
|---|---|---|
144.23KiB |
144.23KiB |
|
31.84KiB |
31.84KiB |
|
0% |
0% |
|
8 |
8 |
|
8 |
8 |
|
219 |
219 |
|
16 |
16 |
|
3.33% |
3.33% |
|
4 |
4 |
|
0 |
0 |
Bundle size by type no changes
| Current #4920 |
Baseline #4907 |
|
|---|---|---|
235.43KiB |
235.43KiB |
|
100.16KiB |
100.16KiB |
|
31.84KiB |
31.84KiB |
Bundle analysis report Branch colinaaa:colin/0908/dedupe Project dashboard
Generated by RelativeCI Documentation Report issue
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (8)
.changeset/real-birds-push.md (1)
5-7: Clarify dedupe behavior and precedence in the changeset.Add brief notes on alias precedence and how unresolved packages are handled to reduce user confusion.
Support `resolve.dedupe`. This is useful when having multiple duplicated packages in the bundle: + +Notes: +- Values from `resolve.dedupe` are implemented via aliases and will be merged with `resolve.alias`; when keys conflict, `resolve.alias` takes precedence. +- If a package can't be resolved from the project root, it is skipped (no error thrown). +- Semantics align with Rsbuild/Vite for ease of migration.packages/rspeedy/core/src/config/resolve/index.ts (2)
94-116: Tighten Resolve.dedupe docs (wording, precedence, unresolved handling).Suggest small wording tweaks and explicitly note unresolved packages are skipped.
- /** - * Force to resolve the specified packages from project root, which is useful for deduplicating packages and reducing the bundle size. + /** + * Force resolving the specified packages from the project root, useful for deduplicating packages and reducing bundle size. @@ - * {@link Resolve.dedupe} is implemented based on {@link Resolve.alias}, it will get the path of the specified package through `require.resolve` in the project root directory and set it to the alias. + * {@link Resolve.dedupe} is implemented on top of {@link Resolve.alias}: it resolves each package via `require.resolve` from the project root and adds it to alias. @@ - * The alias generated by {@link Resolve.dedupe} will be merged with the configured {@link Resolve.alias} in the project, and the {@link Resolve.alias} config will take precedence when the keys are the same. + * Aliases generated by {@link Resolve.dedupe} are merged with {@link Resolve.alias}; when keys conflict, {@link Resolve.alias} takes precedence. + * + * If a package cannot be resolved from the project root, it is ignored.
117-117: Accept readonly arrays to easeas constconfigs.Many users write config arrays with
as const. Consider widening toReadonlyArray<string>. If adopted, update API report and tests accordingly.- dedupe?: string[] | undefined + dedupe?: ReadonlyArray<string> | undefinedpackages/rspeedy/core/test/config/rsbuild.test.ts (1)
583-591: Good coverage for dedupe passthrough.The test verifies
resolve.dedupemapping intoRsbuildConfig. Consider an additional case to ensure alias and dedupe coexist.+ test('transform resolve.alias + dedupe together', () => { + const rsbuildConfig = toRsbuildConfig({ + resolve: { + alias: { foo: 'bar' }, + dedupe: ['baz'], + }, + }) + expect(rsbuildConfig.resolve?.alias).toStrictEqual({ foo: 'bar' }) + expect(rsbuildConfig.resolve?.dedupe).toStrictEqual(['baz']) + })packages/rspeedy/core/test/config/resolve/dedupe.test-d.ts (1)
11-21: Optionally support readonly arrays to improve DX.If you widen the type to
ReadonlyArray<string>, add this assertion to coveras const.assertType<Resolve>({ dedupe: ['foo', 'bar'], }) + assertType<Resolve>({ + // supports as-const tuples when using ReadonlyArray<string> + dedupe: ['foo', 'bar'] as const, + })packages/rspeedy/core/test/config/resolve/dedupe.test.ts (1)
24-36: Add a test for alias-overrides-dedupe to lock in precedence.Docs say
resolve.aliaswins when keys clash. Add a case asserting alias value is preserved when both specify the same package.test('resolve.dedupe with string[]', async () => { @@ }) + + test('resolve.alias should override resolve.dedupe on key conflict', async () => { + const rspeedy = await createStubRspeedy({ + resolve: { + alias: { '@rsbuild/core': 'ALIAS_PATH' }, + dedupe: ['@rsbuild/core'], + }, + }) + const config = await rspeedy.unwrapConfig() + expect(config.resolve?.alias).toHaveProperty('@rsbuild/core', 'ALIAS_PATH') + })packages/rspeedy/core/test/config/validate.test.ts (2)
1788-1800: Add a scoped package example to valid Resolve.dedupe casesInclude a scoped name to mirror real-world usage and the docs example.
const resolveCases: Resolve[] = [ {}, { alias: {} }, { alias: { foo: 'bar', bar$: 'baz', baz: false } }, { alias: { foo: 'bar', bar$: ['baz'] } }, { dedupe: [] }, + { dedupe: ['@lynx-js/react'] }, { dedupe: ['foo'] }, { dedupe: ['foo', 'bar', 'baz'] }, ]
1932-1953: Add top-level string invalid case for dedupeYou already cover invalid element types; add a simple non-array type to close the gap.
expect(() => validate({ resolve: { dedupe: {}, }, }) ).toThrowErrorMatchingInlineSnapshot(` [Error: Invalid configuration. Invalid config on \`$input.resolve.dedupe\`. - Expect to be (Array<string> | undefined) - Got: object ] `) + expect(() => + validate({ + resolve: { + dedupe: 'foo', + }, + }) + ).toThrowErrorMatchingInlineSnapshot(` + [Error: Invalid configuration. + + Invalid config on \`$input.resolve.dedupe\`. + - Expect to be (Array<string> | undefined) + - Got: string + ] + `)
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
.changeset/real-birds-push.md(1 hunks)packages/rspeedy/core/etc/rspeedy.api.md(1 hunks)packages/rspeedy/core/src/config/resolve/index.ts(1 hunks)packages/rspeedy/core/src/config/rsbuild/index.ts(1 hunks)packages/rspeedy/core/test/config/resolve/alias.test.ts(1 hunks)packages/rspeedy/core/test/config/resolve/dedupe.test-d.ts(1 hunks)packages/rspeedy/core/test/config/resolve/dedupe.test.ts(1 hunks)packages/rspeedy/core/test/config/rsbuild.test.ts(1 hunks)packages/rspeedy/core/test/config/validate.test.ts(3 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
packages/**/etc/*.api.md
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
packages/**/etc/*.api.md: After running API extractor, commit updated API report files
Checketc/*.api.mdfiles when API changes are needed
Files:
packages/rspeedy/core/etc/rspeedy.api.md
🧠 Learnings (2)
📚 Learning: 2025-08-06T13:28:57.182Z
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1453
File: vitest.config.ts:49-61
Timestamp: 2025-08-06T13:28:57.182Z
Learning: In the lynx-family/lynx-stack repository, the file `packages/rspeedy/create-rspeedy/template-react-vitest-rltl-js/vitest.config.js` is a template file for scaffolding new Rspeedy projects, not a test configuration that should be included in the main vitest projects array.
Applied to files:
packages/rspeedy/core/test/config/resolve/dedupe.test.ts
📚 Learning: 2025-07-22T09:23:07.797Z
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1330
File: .changeset/olive-animals-attend.md:1-3
Timestamp: 2025-07-22T09:23:07.797Z
Learning: In the lynx-family/lynx-stack repository, changesets are only required for meaningful changes to end-users such as bugfixes and features. Internal/development changes like chores, refactoring, or removing debug info do not need changeset entries.
Applied to files:
.changeset/real-birds-push.md
🧬 Code graph analysis (3)
packages/rspeedy/core/test/config/resolve/dedupe.test-d.ts (1)
packages/rspeedy/core/src/config/resolve/index.ts (1)
Resolve(10-118)
packages/rspeedy/core/test/config/rsbuild.test.ts (1)
packages/rspeedy/core/src/config/rsbuild/index.ts (1)
toRsbuildConfig(15-123)
packages/rspeedy/core/test/config/validate.test.ts (2)
packages/rspeedy/core/src/config/resolve/index.ts (1)
Resolve(10-118)packages/rspeedy/core/src/config/validate.ts (1)
validate(16-48)
🔇 Additional comments (6)
packages/rspeedy/core/test/config/resolve/alias.test.ts (1)
9-9: Suite rename improves specificity.Renaming to "Config - Resolve.alias" is accurate and keeps scope clear alongside the new dedupe tests.
packages/rspeedy/core/etc/rspeedy.api.md (1)
267-268: API report updated correctly for Resolve.dedupe.The public surface reflects
dedupe?: string[] | undefined. Remember to re-run API Extractor if you adjust the type toReadonlyArray<string>.packages/rspeedy/core/test/config/resolve/dedupe.test-d.ts (1)
9-26: Type tests look correct and focused.Covers undefined, empty, and array cases; rejects object.
packages/rspeedy/core/src/config/rsbuild/index.ts (1)
58-61: Pass-through of resolve.dedupe is correct.Mapping
config.resolve?.dedupetoresolve.dedupein Rsbuild config looks good and matches tests.packages/rspeedy/core/test/config/validate.test.ts (2)
15-15: LGTM: public Resolve type import is correctImporting
Resolvefrom the public API matches the PR intent and keeps tests aligned with exported types.
1917-1930: LGTM: rejects non-array dedupe input (object) with precise error pathGood negative coverage and message shape.
CodSpeed Performance ReportMerging #1671 will not alter performanceComparing 🎉 Hooray!
|
upupming
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
This PR was opened by the [Changesets release](https://github.com/changesets/action) GitHub action. When you're ready to do a release, you can merge this and the packages will be published to npm automatically. If you're not ready to do a release yet, that's fine, whenever you add more changesets to main, this PR will be updated. # Releases ## @lynx-js/[email protected] ### Minor Changes - fix: Delay execution of `runOnMainThread()` during initial render ([#1667](#1667)) When called during the initial render, `runOnMainThread()` would execute before the `main-thread:ref` was hydrated, causing it to be incorrectly set to null. This change delays the function's execution to ensure the ref is available and correctly assigned. ### Patch Changes - Fix "TypeError: cannot read property '0' of undefined" in deferred list-item scenarios. ([#1692](#1692)) Deferred `componentAtIndex` causes nodes that quickly appear/disappear to be enqueued without `__elements`. Update `signMap` before `__FlushElementTree` to resolve the issue. - Keep the same `<page/>` element when calling `rerender` in testing library. ([#1656](#1656)) - Bump `swc_core` to `39.0.3`. ([#1721](#1721)) ## @lynx-js/[email protected] ### Minor Changes - Added `group-*`, `peer-*`, and `parent-*` modifiers (ancestor, sibling, and direct-parent scopes) for `uiVariants` plugin. ([#1741](#1741)) Fixed prefix handling in prefixed projects — `ui-*` state markers are not prefixed, while scope markers (`.group`/`.peer`) honor `config('prefix')`. **BREAKING**: Removed slash-based naming modifiers on self (non-standard); slash modifiers remain supported for scoped markers (e.g. `group/menu`, `peer/tab`). Bumped peer dependency to `tailwindcss@^3.4.0` (required for use of internal features). ## @lynx-js/[email protected] ### Minor Changes - Remove `@lynx-js/react` from peerDependencies. ([#1711](#1711)) - Add a new required option `workletRuntimePath`. ([#1711](#1711)) ## @lynx-js/[email protected] ### Patch Changes - Support `server.proxy`. ([#1745](#1745)) - Support `command` and `env` parameters in the function exported by `lynx.config.js`. ([#1669](#1669)) ```js import { defineConfig } from "@lynx-js/rspeedy"; export default defineConfig(({ command, env }) => { const isBuild = command === "build"; const isTest = env === "test"; return { output: { minify: !isTest, }, performance: { buildCache: isBuild, }, }; }); ``` - Support `resolve.dedupe`. ([#1671](#1671)) This is useful when having multiple duplicated packages in the bundle: ```js import { defineConfig } from "@lynx-js/rspeedy"; export default defineConfig({ resolve: { dedupe: ["tslib"], }, }); ``` - Support `resolve.aliasStrategy` for controlling priority between `tsconfig.json` paths and `resolve.alias` ([#1722](#1722)) ```js import { defineConfig } from "@lynx-js/rspeedy"; export default defineConfig({ resolve: { alias: { "@": "./src", }, // 'prefer-tsconfig' (default): tsconfig.json paths take priority // 'prefer-alias': resolve.alias takes priority aliasStrategy: "prefer-alias", }, }); ``` - Bump Rsbuild v1.5.4 with Rspack v1.5.2. ([#1644](#1644)) - Updated dependencies \[[`d7c5da3`](d7c5da3)]: - @lynx-js/[email protected] - @lynx-js/[email protected] ## @lynx-js/[email protected] ### Patch Changes - Fix using wrong version of `@lynx-js/react/worklet-runtime`. ([#1711](#1711)) - Be compat with `@lynx-js/react` v0.113.0 ([#1667](#1667)) - Disable `builtin:lightningcss-loader` for `environments.web`. ([#1732](#1732)) - Updated dependencies \[[`5ad38e6`](5ad38e6), [`69b3ae0`](69b3ae0), [`69b3ae0`](69b3ae0), [`c2f90bd`](c2f90bd)]: - @lynx-js/[email protected] - @lynx-js/[email protected] - @lynx-js/[email protected] - @lynx-js/[email protected] - @lynx-js/[email protected] - @lynx-js/[email protected] ## @lynx-js/[email protected] ### Patch Changes - Allow customization of the react$ alias. ([#1653](#1653)) ```js import { defineConfig } from "@lynx-js/rspeedy"; export default defineConfig({ resolve: { alias: { react$: "@lynx-js/react/compat", }, }, }); ``` ## @lynx-js/[email protected] ### Patch Changes - feat: supports lazy bundle. (This feature requires `@lynx-js/lynx-core >= 0.1.3`) ([#1235](#1235)) - Updated dependencies \[]: - @lynx-js/[email protected] ## @lynx-js/[email protected] ### Patch Changes - refactor: improve chunk loading ([#1703](#1703)) - feat: supports lazy bundle. (This feature requires `@lynx-js/lynx-core >= 0.1.3`) ([#1235](#1235)) - Updated dependencies \[[`608f375`](608f375)]: - @lynx-js/[email protected] - @lynx-js/[email protected] - @lynx-js/[email protected] - @lynx-js/[email protected] ## @lynx-js/[email protected] ### Patch Changes - refactor: improve chunk loading ([#1703](#1703)) - feat: supports lazy bundle. (This feature requires `@lynx-js/lynx-core >= 0.1.3`) ([#1235](#1235)) ## @lynx-js/[email protected] ### Patch Changes - fix: 1. svg use image tag to render, to differentiate background-image styles ([#1668](#1668)) 1. use blob instead of raw data-uri > Not using data-uri(data:image/svg+xml;utf8,${props.content}) > since it has follow limitations: > > < and > must be encoded to %3C and %3E. > Double quotes must be converted to single quotes. > Colors must use a non-hex format because # will not work inside data-uri. > See: <https://codepen.io/zvuc/pen/BWNLJL> > Instead, we use modern Blob API to create SVG URL that have the same support - Updated dependencies \[[`d618304`](d618304), [`1d97fce`](1d97fce)]: - @lynx-js/[email protected] ## @lynx-js/[email protected] ### Patch Changes - x-overlay-ng prevent page scroll when visible ([#1499](#1499)) - fix: 1. svg use image tag to render, to differentiate background-image styles ([#1668](#1668)) 1. use blob instead of raw data-uri > Not using data-uri(data:image/svg+xml;utf8,${props.content}) > since it has follow limitations: > > < and > must be encoded to %3C and %3E. > Double quotes must be converted to single quotes. > Colors must use a non-hex format because # will not work inside data-uri. > See: <https://codepen.io/zvuc/pen/BWNLJL> > Instead, we use modern Blob API to create SVG URL that have the same support ## @lynx-js/[email protected] ### Patch Changes - feat: supports lazy bundle. (This feature requires `@lynx-js/lynx-core >= 0.1.3`) ([#1235](#1235)) - Updated dependencies \[[`608f375`](608f375)]: - @lynx-js/[email protected] - @lynx-js/[email protected] ## @lynx-js/[email protected] ### Patch Changes - feat: supports lazy bundle. (This feature requires `@lynx-js/lynx-core >= 0.1.3`) ([#1235](#1235)) - Updated dependencies \[[`608f375`](608f375)]: - @lynx-js/[email protected] - @lynx-js/[email protected] - @lynx-js/[email protected] ## @lynx-js/[email protected] ### Patch Changes - Fix unmet peer dependency "@rspack/core@'^1.3.10". ([#1660](#1660)) ## @lynx-js/[email protected] ### Patch Changes - fix: add appType field for lazy bundle for web ([#1738](#1738)) ## [email protected] ## [email protected] ## @lynx-js/[email protected] ## @lynx-js/[email protected] Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Summary by CodeRabbit
New Features
Documentation
Tests
close: #1519
Checklist