Conversation
There was a problem hiding this comment.
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.
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'] =>
meteorlxy
reviewed
May 8, 2025
meteorlxy
approved these changes
May 8, 2025
meteorlxy
approved these changes
May 8, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.