-
Notifications
You must be signed in to change notification settings - Fork 922
fix: make alias can be overridden properly #1648
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
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.
Pull Request Overview
This PR fixes an alias override issue in bundler-vite by ensuring that longer alias keys are prioritized over shorter ones, and also tightens the type definition to support consistent alias behavior between Vite and webpack.
- Updated tests to validate that longer aliases override shorter ones.
- Added module exports and documentation samples to demonstrate the revised alias resolution.
- Updated the VuePress config to include a sorted alias mapping.
Reviewed Changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
e2e/tests/alias/override.spec.ts | Test verifying that longer aliases override shorter ones. |
e2e/tests/alias/dir.spec.ts | Test checking alias resolution for subpaths. |
e2e/modules/dir2/b.js | Added module exporting alias result for dir2/b. |
e2e/modules/dir2/a.js | Added module exporting alias result for dir2/a. |
e2e/modules/dir1/c.js | Added module exporting alias result for dir1/c. |
e2e/modules/dir1/b.js | Added module exporting alias result for dir1/b. |
e2e/modules/dir1/a.js | Added module exporting alias result for dir1/a. |
e2e/docs/alias/override.md | Documentation demonstrating alias override output. |
e2e/docs/alias/dir.md | Documentation demonstrating alias resolution for a subpath. |
e2e/docs/.vuepress/config.ts | Updated alias configuration to enforce correct key ordering. |
Pull Request Test Coverage Report for Build 14909966417Details
💛 - Coveralls |
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.
Pull Request Overview
This PR fixes issues in alias resolution by ensuring that longer alias keys are replaced before shorter ones and by unifying the type definition for alias handling across bundlers. Key changes include:
- Updating type definitions by splitting the old AliasDefineHook into AliasHook and DefineHook.
- Modifying hook normalization and plugin API registration to accept the new generic hook types.
- Adjusting bundler configurations (webpack and vite) to sort aliases by key length and correctly merge alias entries.
Reviewed Changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
packages/core/tests/pluginApi/normalizeAliasDefineHook.spec.ts | Updated test imports and removed duplicate tests for hook normalization. |
packages/core/src/types/pluginApi/hooks.ts | Split AliasDefineHook into distinct AliasHook and DefineHook types. |
packages/core/src/pluginApi/normalizeAliasDefineHook.ts | Updated normalization logic to use a generic over both hook types. |
packages/core/src/pluginApi/createPluginApiRegisterHooks.ts | Updated hook registration calls with explicit generics. |
packages/bundler-webpack/src/config/handleResolve.ts | Refactored alias merging with sorting by key length and streamlined alias hook processing. |
packages/bundler-vite/src/plugins/vuepressConfigPlugin.ts | Updated alias processing to use Object.assign and sort alias entries. |
e2e tests and docs | Added tests and documentation to verify and illustrate the new alias override behavior. |
Comments suppressed due to low confidence (1)
packages/core/src/pluginApi/normalizeAliasDefineHook.ts:10
- [nitpick] The function name 'normalizeAliasDefineHook' now handles both alias and define hooks; consider renaming it (e.g., to 'normalizeConfigHook') to better reflect its broader purpose.
<T extends AliasHook | DefineHook>(hook: T['exposed']): T['normalized'] =>
What is the purpose of this pull request?
Description
Vite/Webpack current has issues handling alias, alias resolution may differ with how they defined.
Users may fail to override some subpaths if a parent folder alias was defined by plugins or themes.
This PR solves the problem by sorting alias key with its length first to ensure longer alias being replaced first.
Also, though webpack support alias as
string[] | false
, Vite can not support these things, so to ensure alias is working in both bundlers, the type definition is restricted toRecord<string, string>
. Users are still allowed to add advanced definition with bundler options.