-
-
Notifications
You must be signed in to change notification settings - Fork 955
feat(virtualModulePrefix): add support for custom virtual module prefix #4958
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
feat(virtualModulePrefix): add support for custom virtual module prefix #4958
Conversation
✅ Deploy Preview for unocss ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify project configuration. |
01794ba to
29ac6d6
Compare
|
commit: |
29ac6d6 to
50067ec
Compare
…com/liujiayii/unocss into feature/support-virtualModulePrefix
不再需要virtualModulePrefix字段,简化配置接口
|
这个啥时候能合并啊 |
|
都一个月了诶 |
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 adds support for custom virtual module prefixes in UnoCSS, allowing users to configure the prefix used for virtual CSS modules instead of being hardcoded to __uno. The feature introduces a new virtualModulePrefix option that can be configured at the framework integration level.
Key Changes:
- Introduced a
ResolvedIdRegexesclass with caching to dynamically generate regex patterns based on custom prefixes - Added
virtualModulePrefixoption to theUserOnlyOptionsinterface - Updated all integration points (Vite, Webpack, Astro) to call
ResolvedIdRegexes.set()with the configured prefix
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| virtual-shared/integration/src/constants.ts | Implements the ResolvedIdRegexes class that replaces hardcoded regexes with dynamically generated ones based on the prefix |
| virtual-shared/integration/src/layers.ts | Updates resolveId and resolveLayer functions to use dynamic regexes and prefix from ResolvedIdRegexes |
| virtual-shared/integration/test/resolved-id-regexes.test.ts | Adds test coverage for the new ResolvedIdRegexes class functionality |
| packages-integrations/webpack/src/unplugin.ts | Integrates prefix initialization for Webpack plugin |
| packages-integrations/vite/src/modes/per-module.ts | Integrates prefix initialization for Vite per-module mode |
| packages-integrations/vite/src/modes/global/dev.ts | Integrates prefix initialization for Vite global dev mode |
| packages-integrations/vite/src/modes/global/build.ts | Integrates prefix initialization for Vite global build mode |
| packages-integrations/astro/src/index.ts | Integrates prefix initialization for Astro integration |
| packages-engine/core/src/types.ts | Adds the virtualModulePrefix configuration option to the type definitions |
| examples/vite-vue3/vite.config.ts | Demonstrates the new feature with a custom prefix in Vue3 example |
| examples/vite-react/vite.config.ts | Demonstrates the new feature with a custom prefix in React example |
| examples/astro-vue/astro.config.ts | Demonstrates the new feature with a custom prefix in Astro example |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
|
|
||
| static set(prefix: string = this._defaultPrefix): void { | ||
| if (typeof prefix !== 'string') { |
Copilot
AI
Nov 28, 2025
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.
The validation logic is incomplete. The error message says "prefix must be a non-empty string", but the code only checks if the type is a string. It doesn't validate that the string is non-empty. An empty string would pass this check but likely cause issues with the regex patterns. Add a check: if (typeof prefix !== 'string' || prefix === '') { throw new TypeError('prefix must be a non-empty string') }
| if (typeof prefix !== 'string') { | |
| if (typeof prefix !== 'string' || prefix === '') { |
| RESOLVED_ID_WITH_QUERY_RE: new RegExp(`[/\\\\]${prefix}(_.*?)?\\.css(\\?.*)?$`), | ||
| RESOLVED_ID_RE: new RegExp(`[/\\\\]${prefix}(?:_(.*?))?\\.css$`), |
Copilot
AI
Nov 28, 2025
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.
Special regex characters in the prefix are not escaped before being used in the RegExp constructor. If a user provides a prefix containing regex metacharacters (e.g., __uno.test, __uno*, __uno[foo]), it will create an invalid or unexpected regex pattern. Use a regex escape function or validate that the prefix only contains safe characters (e.g., alphanumeric and underscores).
| injectExtra = [], | ||
| } = options | ||
|
|
||
| ResolvedIdRegexes.set(options.virtualModulePrefix) |
Copilot
AI
Nov 28, 2025
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.
The ResolvedIdRegexes.set() is called synchronously in the Astro integration initialization, but it may be called before the config is fully resolved. Unlike the Vite integrations which wait for ctx.ready before setting the prefix, this code doesn't wait. This could lead to race conditions where the regex is set with undefined before the config is ready. Consider waiting for config resolution or adding a check for undefined values.
| timer = setTimeout(updateModules, UPDATE_DEBOUNCE) | ||
| }) | ||
|
|
||
| ResolvedIdRegexes.set(ctx.uno.config.virtualModulePrefix) |
Copilot
AI
Nov 28, 2025
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.
The ResolvedIdRegexes.set() is called synchronously during plugin initialization before ctx.ready is awaited, but it accesses ctx.uno.config.virtualModulePrefix. According to the context implementation, accessing ctx.uno before awaiting ctx.ready will throw an error: "Run await context.ready before accessing context.uno". This should be moved to after ctx.ready is resolved (e.g., inside the beforeCompile hook where ctx.ready is already awaited).
| await ctx.ready | ||
| ResolvedIdRegexes.set(ctx.uno.config.virtualModulePrefix) |
Copilot
AI
Nov 28, 2025
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.
The ResolvedIdRegexes.set() call is placed in the load hook, but the resolveId hook (which executes earlier in the Vite plugin lifecycle) calls resolveId() from layers.ts at line 40, which in turn calls ResolvedIdRegexes.get(). This creates a timing issue where the regex patterns might be retrieved before the custom prefix is set, causing the plugin to use the default __uno prefix initially. Consider moving the ResolvedIdRegexes.set() call to an earlier lifecycle hook that runs once (e.g., configResolved).
| /** | ||
| * Custom prefix for virtual modules | ||
| * | ||
| * @default '__uno' |
Copilot
AI
Nov 28, 2025
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.
[nitpick] The JSDoc comment should use backticks around code values for consistency. Change @default '__uno' to @default '__uno' (using code formatting).
| * @default '__uno' | |
| * @default `__uno` |
| private static cache = new Map<string, { RESOLVED_ID_WITH_QUERY_RE: RegExp, RESOLVED_ID_RE: RegExp }>() | ||
| private static _defaultPrefix = '__uno' | ||
| private static _currentPrefix: string | undefined | ||
|
|
||
| private static createRegexes(prefix: string): { RESOLVED_ID_WITH_QUERY_RE: RegExp, RESOLVED_ID_RE: RegExp } { | ||
| return { | ||
| RESOLVED_ID_WITH_QUERY_RE: new RegExp(`[/\\\\]${prefix}(_.*?)?\\.css(\\?.*)?$`), | ||
| RESOLVED_ID_RE: new RegExp(`[/\\\\]${prefix}(?:_(.*?))?\\.css$`), | ||
| } | ||
| } | ||
|
|
||
| static get(): { RESOLVED_ID_WITH_QUERY_RE: RegExp, RESOLVED_ID_RE: RegExp } { | ||
| const prefix = this._currentPrefix || this._defaultPrefix | ||
|
|
||
| if (!this.cache.has(prefix)) { | ||
| this.cache.set(prefix, this.createRegexes(prefix)) | ||
| } | ||
|
|
||
| return this.cache.get(prefix)! | ||
| } | ||
|
|
||
| static set(prefix: string = this._defaultPrefix): void { | ||
| if (typeof prefix !== 'string') { | ||
| throw new TypeError('prefix must be a non-empty string') | ||
| } | ||
| this.cache.set(prefix, this.createRegexes(prefix)) | ||
| // 设置当前使用的前缀 | ||
| this._currentPrefix = prefix | ||
| } | ||
|
|
||
| static currentPrefix(): string { |
Copilot
AI
Nov 28, 2025
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.
The ResolvedIdRegexes class uses static state (_currentPrefix and cache) which is shared across all instances of the plugin. This can cause issues in scenarios where multiple UnoCSS configurations with different virtualModulePrefix values need to coexist (e.g., in monorepos or when running multiple builds concurrently). Consider refactoring to make the regex patterns instance-based or pass them through the context object instead of using global static state.
| private static cache = new Map<string, { RESOLVED_ID_WITH_QUERY_RE: RegExp, RESOLVED_ID_RE: RegExp }>() | |
| private static _defaultPrefix = '__uno' | |
| private static _currentPrefix: string | undefined | |
| private static createRegexes(prefix: string): { RESOLVED_ID_WITH_QUERY_RE: RegExp, RESOLVED_ID_RE: RegExp } { | |
| return { | |
| RESOLVED_ID_WITH_QUERY_RE: new RegExp(`[/\\\\]${prefix}(_.*?)?\\.css(\\?.*)?$`), | |
| RESOLVED_ID_RE: new RegExp(`[/\\\\]${prefix}(?:_(.*?))?\\.css$`), | |
| } | |
| } | |
| static get(): { RESOLVED_ID_WITH_QUERY_RE: RegExp, RESOLVED_ID_RE: RegExp } { | |
| const prefix = this._currentPrefix || this._defaultPrefix | |
| if (!this.cache.has(prefix)) { | |
| this.cache.set(prefix, this.createRegexes(prefix)) | |
| } | |
| return this.cache.get(prefix)! | |
| } | |
| static set(prefix: string = this._defaultPrefix): void { | |
| if (typeof prefix !== 'string') { | |
| throw new TypeError('prefix must be a non-empty string') | |
| } | |
| this.cache.set(prefix, this.createRegexes(prefix)) | |
| // 设置当前使用的前缀 | |
| this._currentPrefix = prefix | |
| } | |
| static currentPrefix(): string { | |
| private cache = new Map<string, { RESOLVED_ID_WITH_QUERY_RE: RegExp, RESOLVED_ID_RE: RegExp }>() | |
| private _defaultPrefix = '__uno' | |
| private _currentPrefix: string | |
| constructor(prefix: string = '__uno') { | |
| if (typeof prefix !== 'string' || !prefix) { | |
| throw new TypeError('prefix must be a non-empty string') | |
| } | |
| this._currentPrefix = prefix | |
| this.cache.set(prefix, this.createRegexes(prefix)) | |
| } | |
| private createRegexes(prefix: string): { RESOLVED_ID_WITH_QUERY_RE: RegExp, RESOLVED_ID_RE: RegExp } { | |
| return { | |
| RESOLVED_ID_WITH_QUERY_RE: new RegExp(`[/\\\\]${prefix}(_.*?)?\\.css(\\?.*)?$`), | |
| RESOLVED_ID_RE: new RegExp(`[/\\\\]${prefix}(?:_(.*?))?\\.css$`), | |
| } | |
| } | |
| get(): { RESOLVED_ID_WITH_QUERY_RE: RegExp, RESOLVED_ID_RE: RegExp } { | |
| const prefix = this._currentPrefix || this._defaultPrefix | |
| if (!this.cache.has(prefix)) { | |
| this.cache.set(prefix, this.createRegexes(prefix)) | |
| } | |
| return this.cache.get(prefix)! | |
| } | |
| set(prefix: string = this._defaultPrefix): void { | |
| if (typeof prefix !== 'string' || !prefix) { | |
| throw new TypeError('prefix must be a non-empty string') | |
| } | |
| this.cache.set(prefix, this.createRegexes(prefix)) | |
| this._currentPrefix = prefix | |
| } | |
| currentPrefix(): string { |
| throw new TypeError('prefix must be a non-empty string') | ||
| } | ||
| this.cache.set(prefix, this.createRegexes(prefix)) | ||
| // 设置当前使用的前缀 |
Copilot
AI
Nov 28, 2025
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.
This comment is in Chinese. Should be in English for consistency with the codebase: "// 设置当前使用的前缀" → "// Set the current prefix"
| // 设置当前使用的前缀 | |
| // Set the current prefix |
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.
老哥,修下再合并下呗
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.
老哥,修下再合并下呗
明天我认真review下 会合并发版
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.
老哥,修下再合并下呗
明天我认真review下 会合并发版
太感谢了,我 crxjs 项目一直等着修复这个问题
close #4843