-
-
Notifications
You must be signed in to change notification settings - Fork 20
feat: 支持多 Resolver 以及 extraConfig 合并 #138
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
WalkthroughAdds an injectUtils module (deepMerge, addResolver, mergeExtraConfig) and PluginData type; renderTemplate now supplies injectUtils as utils to .data.mjs getData. Multiple template data files update getData signatures to accept utils, remove inline initializer strings, inject resolvers via utils.addResolver, and standardize extraConfig merging via utils.mergeExtraConfig/deepMerge. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as Caller
participant RT as renderTemplate
participant DF as *.data.mjs (getData)
participant U as injectUtils
participant AR as addResolver
participant MM as mergeExtraConfig/deepMerge
Dev->>RT: render(template, dataStore)
RT->>DF: getData({ oldData, utils: injectUtils })
note over DF,U: getData may call utils.addResolver or utils.mergeExtraConfig
DF->>AR: utils.addResolver(plugin, "XResolver()")
AR-->>DF: plugin' (resolver injected into initializer string)
DF->>MM: utils.mergeExtraConfig(oldExtra, newExtra)
MM-->>DF: mergedExtra
DF-->>RT: { plugins, extraConfig, ... }
RT-->>Dev: rendered output
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used🧬 Code graph analysis (4)packages/core/template/module/uniUse/vite.config.js.data.mjs (4)
packages/core/template/UI/uview-pro/vite.config.js.data.mjs (2)
packages/core/src/utils/injectUtils.ts (2)
packages/core/template/UI/nut/vite.config.js.data.mjs (7)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)
🔇 Additional comments (5)
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. 🧪 Early access (Sonnet 4.5): enabledWe are currently testing the Sonnet 4.5 model, which is expected to improve code review quality. However, this model may lead to increased noise levels in the review comments. Please disable the early access features if the noise level causes any inconvenience. Note:
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.
Actionable comments posted: 1
🧹 Nitpick comments (3)
packages/core/template/base/vite.config.js.data.mjs (1)
1-12: Formatting change looks good, but consider adopting utils-based pattern.The indentation adjustment is fine. However, unlike other templates updated in this PR, this
getDatafunction doesn't accept the newutilsparameter. While the base template may not currently need resolvers or deep merging, consider updating the signature togetData({ oldData, utils })for consistency across templates, even ifutilsisn't used yet.packages/core/src/utils/types.ts (1)
1-6: Consider adding JSDoc documentation.The
PluginDatainterface is well-structured with clear field names. Consider adding JSDoc comments to document the purpose of each field and provide usage examples, especially since this is a new public type that will be used across multiple templates.Example:
/** * Metadata for a Vite plugin configuration. */ export interface PluginData { /** Unique identifier for the plugin */ id: string /** Static import statement (e.g., "import Foo from 'foo'") */ importer?: string /** Dynamic import statement */ dynamicImporter?: string /** Plugin initialization code */ initializer?: string }packages/core/template/plugin/import/vite.config.js.data.mjs (1)
1-18: Add empty resolvers array enables future injection, but consider adopting utils parameter.The addition of the empty
resolvers: []array on line 8 is essential—it provides the target pattern thataddResolverrequires (without it,addResolverwould silently fail, as noted in theinjectUtils.tsreview).However, this
getDatafunction doesn't yet accept theutilsparameter that other templates in this PR are adopting. Consider updating the signature togetData({ oldData, utils })for consistency, even if this specific template doesn't directly callutils.addResolverorutils.deepMerge.-export default function getData({ oldData }) { +export default function getData({ oldData, utils }) { const autoImportPlugin = {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (14)
packages/core/src/utils/injectUtils.ts(1 hunks)packages/core/src/utils/renderTemplate.ts(2 hunks)packages/core/src/utils/types.ts(1 hunks)packages/core/template/UI/ano/vite.config.js.data.mjs(1 hunks)packages/core/template/UI/nut/vite.config.js.data.mjs(2 hunks)packages/core/template/UI/skiyee/vite.config.js.data.mjs(1 hunks)packages/core/template/UI/uni/vite.config.js.data.mjs(1 hunks)packages/core/template/UI/uv/vite.config.js.data.mjs(1 hunks)packages/core/template/UI/uview-pro/vite.config.js.data.mjs(2 hunks)packages/core/template/UI/wot/vite.config.js.data.mjs(1 hunks)packages/core/template/base/vite.config.js.data.mjs(1 hunks)packages/core/template/module/uniPromises/vite.config.js.data.mjs(2 hunks)packages/core/template/module/uniUse/vite.config.js.data.mjs(2 hunks)packages/core/template/plugin/import/vite.config.js.data.mjs(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (11)
packages/core/template/UI/uv/vite.config.js.data.mjs (2)
packages/core/template/UI/ano/vite.config.js.data.mjs (1)
getData(1-15)packages/core/template/plugin/import/vite.config.js.data.mjs (1)
getData(1-18)
packages/core/template/UI/ano/vite.config.js.data.mjs (7)
packages/core/template/UI/nut/vite.config.js.data.mjs (1)
getData(1-26)packages/core/template/UI/skiyee/vite.config.js.data.mjs (1)
getData(1-15)packages/core/template/UI/uni/vite.config.js.data.mjs (1)
getData(1-18)packages/core/template/UI/uv/vite.config.js.data.mjs (1)
getData(1-15)packages/core/template/UI/uview-pro/vite.config.js.data.mjs (1)
getData(1-26)packages/core/template/UI/wot/vite.config.js.data.mjs (1)
getData(1-15)packages/core/template/plugin/import/vite.config.js.data.mjs (1)
getData(1-18)
packages/core/src/utils/renderTemplate.ts (1)
packages/core/src/utils/injectUtils.ts (1)
injectUtils(22-25)
packages/core/template/UI/uni/vite.config.js.data.mjs (4)
packages/core/template/UI/ano/vite.config.js.data.mjs (1)
getData(1-15)packages/core/template/UI/nut/vite.config.js.data.mjs (1)
getData(1-26)packages/core/template/UI/uview-pro/vite.config.js.data.mjs (1)
getData(1-26)packages/core/template/plugin/import/vite.config.js.data.mjs (1)
getData(1-18)
packages/core/template/UI/nut/vite.config.js.data.mjs (3)
packages/core/template/UI/ano/vite.config.js.data.mjs (1)
getData(1-15)packages/core/template/UI/uview-pro/vite.config.js.data.mjs (1)
getData(1-26)packages/core/template/plugin/import/vite.config.js.data.mjs (1)
getData(1-18)
packages/core/template/UI/wot/vite.config.js.data.mjs (7)
packages/core/template/UI/ano/vite.config.js.data.mjs (1)
getData(1-15)packages/core/template/UI/nut/vite.config.js.data.mjs (1)
getData(1-26)packages/core/template/UI/skiyee/vite.config.js.data.mjs (1)
getData(1-15)packages/core/template/UI/uni/vite.config.js.data.mjs (1)
getData(1-18)packages/core/template/UI/uv/vite.config.js.data.mjs (1)
getData(1-15)packages/core/template/UI/uview-pro/vite.config.js.data.mjs (1)
getData(1-26)packages/core/template/plugin/import/vite.config.js.data.mjs (1)
getData(1-18)
packages/core/template/module/uniUse/vite.config.js.data.mjs (3)
packages/core/template/UI/nut/vite.config.js.data.mjs (1)
getData(1-26)packages/core/template/UI/uview-pro/vite.config.js.data.mjs (1)
getData(1-26)packages/core/template/module/uniPromises/vite.config.js.data.mjs (1)
getData(1-16)
packages/core/src/utils/injectUtils.ts (1)
packages/core/src/utils/types.ts (1)
PluginData(1-6)
packages/core/template/module/uniPromises/vite.config.js.data.mjs (4)
packages/core/template/UI/nut/vite.config.js.data.mjs (1)
getData(1-26)packages/core/template/UI/uview-pro/vite.config.js.data.mjs (1)
getData(1-26)packages/core/template/base/vite.config.js.data.mjs (1)
getData(1-12)packages/core/template/module/uniUse/vite.config.js.data.mjs (1)
getData(1-16)
packages/core/template/UI/uview-pro/vite.config.js.data.mjs (8)
packages/core/template/UI/ano/vite.config.js.data.mjs (1)
getData(1-15)packages/core/template/UI/nut/vite.config.js.data.mjs (1)
getData(1-26)packages/core/template/UI/uni/vite.config.js.data.mjs (1)
getData(1-18)packages/core/template/UI/uv/vite.config.js.data.mjs (1)
getData(1-15)packages/core/template/UI/wot/vite.config.js.data.mjs (1)
getData(1-15)packages/core/template/base/vite.config.js.data.mjs (1)
getData(1-12)packages/core/template/module/uniPromises/vite.config.js.data.mjs (1)
getData(1-16)packages/core/template/plugin/import/vite.config.js.data.mjs (1)
getData(1-18)
packages/core/template/UI/skiyee/vite.config.js.data.mjs (7)
packages/core/template/UI/ano/vite.config.js.data.mjs (1)
getData(1-15)packages/core/template/UI/nut/vite.config.js.data.mjs (1)
getData(1-26)packages/core/template/UI/uni/vite.config.js.data.mjs (1)
getData(1-18)packages/core/template/UI/uv/vite.config.js.data.mjs (1)
getData(1-15)packages/core/template/UI/uview-pro/vite.config.js.data.mjs (1)
getData(1-26)packages/core/template/UI/wot/vite.config.js.data.mjs (1)
getData(1-15)packages/core/template/plugin/import/vite.config.js.data.mjs (1)
getData(1-18)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)
- GitHub Check: test (--ts, -e, -p import -p pages -p layouts -p manifest -p filePlatform -p platformModifier -p ...
- GitHub Check: test (--ts, -e, -p import -p pages -p layouts -p manifest -p filePlatform -p platformModifier -p ...
- GitHub Check: test (--ts, -e, -p import -p pages -p layouts -p manifest -p filePlatform -p platformModifier -p ...
- GitHub Check: test (--ts, -e, -p import -p pages -p layouts -p manifest -p filePlatform -p platformModifier -p ...
- GitHub Check: test (--ts, -e, -p import -p pages -p layouts -p manifest -p filePlatform -p platformModifier -p ...
- GitHub Check: test (-e, -p import -p pages -p layouts -p manifest -p filePlatform -p platformModifier -p middle...
- GitHub Check: test (-e, -p import -p pages -p layouts -p manifest -p filePlatform -p platformModifier -p middle...
- GitHub Check: test (-e, -p import -p pages -p layouts -p manifest -p filePlatform -p platformModifier -p middle...
- GitHub Check: test (-e, -p import -p pages -p layouts -p manifest -p filePlatform -p platformModifier -p middle...
- GitHub Check: test (-e, -p import -p pages -p layouts -p manifest -p filePlatform -p platformModifier -p middle...
- GitHub Check: test (-e, -p import -p pages -p layouts -p manifest -p filePlatform -p platformModifier -p middle...
- GitHub Check: test (-e, -p import -p pages -p layouts -p manifest -p filePlatform -p platformModifier -p middle...
- GitHub Check: test (-e, -p import -p pages -p layouts -p manifest -p filePlatform -p platformModifier -p middle...
- GitHub Check: test (-t vitesse, 20.x, windows-latest)
- GitHub Check: test (-t vitesse, 20.x, macos-latest)
- GitHub Check: stable - aarch64-pc-windows-msvc - node@20
- GitHub Check: stable - aarch64-apple-darwin - node@20
- GitHub Check: stable - x86_64-apple-darwin - node@20
- GitHub Check: stable - i686-pc-windows-msvc - node@20
- GitHub Check: stable - x86_64-pc-windows-msvc - node@20
🔇 Additional comments (24)
packages/core/src/utils/injectUtils.ts (3)
1-2: LGTM!Imports are clean and correctly structured, with type-only import for
PluginData.
11-18: Verify silent failure behavior is acceptable.If the
initializerdoesn't contain aresolvers: [...]pattern, thereplace()call returns the original string unchanged with no error or warning. This silent failure could make debugging difficult whenaddResolveris called on an initializer that doesn't have the expected structure.Consider adding validation or a warning when the pattern isn't found.
const modified = plugin.initializer.replace( /(resolvers\s*:\s*\[)([\s\S]*?)(\])/, (_, prefix, content, suffix) => { const cont = content.trim() return `${prefix}${cont}${cont ? ', ' : ''}${resolver}${suffix}` }, ) // If pattern not found, replace returns original if (modified === plugin.initializer && !plugin.initializer.includes('resolvers')) { console.warn(`addResolver: no resolvers array found in initializer for plugin "${plugin.id}"`) } return { ...plugin, initializer: modified, }
22-25: LGTM!The
injectUtilsexport cleanly exposes both utilities for consumption by template data functions.packages/core/src/utils/renderTemplate.ts (2)
6-6: LGTM!The import is correctly placed and enables the utils injection functionality.
79-82: LGTM!Passing
injectUtilsasutilsto thegetDatacallback cleanly enables all template data functions to access the new utilities while maintaining backward compatibility with templates that don't use them yet.packages/core/template/module/uniPromises/vite.config.js.data.mjs (2)
1-1: LGTM: Function signature updated to support utilities.The signature change enables access to the new
utilsparameter containingdeepMergeandaddResolverhelpers, consistent with the PR's objective to support multiple resolvers and improved config merging.
14-14: LGTM: Deep merge correctly handles nested configuration.The
utils.deepMergecall properly handles deep merging of nested config objects (build, optimizeDeps), which is superior to the previous shallow spread approach. The defensive check foroldData.extraConfigis appropriate.packages/core/template/module/uniUse/vite.config.js.data.mjs (2)
1-1: LGTM: Signature update consistent with PR pattern.Function signature updated to receive
utilsparameter, enabling access to utility methods for config handling.
14-14: LGTM: Proper deep merge for nested Vite configuration.Correctly uses
utils.deepMergeto handle nested build and optimizeDeps configuration, with appropriate null-safety check.packages/core/template/UI/wot/vite.config.js.data.mjs (2)
1-1: LGTM: Signature updated for utility access.Consistent with the PR's pattern of adding
utilsparameter togetDatafunctions.
9-13: LGTM: Resolver injection via utils.addResolver.The
flatMaptransformation correctly identifies theautoImportplugin and injects the WotResolver usingutils.addResolver, then adds the companion import plugin. This centralized approach is cleaner than inline initializers.packages/core/template/UI/uv/vite.config.js.data.mjs (2)
1-1: LGTM: Signature update for utility access.Function signature updated to receive
utilsparameter, consistent with the PR pattern.
9-13: LGTM: Consistent resolver injection pattern.Correctly applies the
utils.addResolverpattern to injectUvResolverinto the autoImport plugin, consistent with other UI template files in this PR.packages/core/template/UI/uni/vite.config.js.data.mjs (2)
1-1: LGTM: Signature updated for utility methods.Function signature updated to accept
utilsparameter, enabling resolver injection capabilities.
12-16: LGTM: Conditional resolver options correctly applied.The resolver injection correctly handles conditional options based on the presence of a root plugin. The
resolverOptionstemplate literal interpolation ensuresUniUIResolverreceives appropriate exclude options when needed, maintaining existing behavior while using the new utility pattern.packages/core/template/UI/skiyee/vite.config.js.data.mjs (2)
1-1: LGTM! Signature updated to support utilities.The function signature now accepts the
utilsparameter, enabling the use of centralized resolver injection and config merging utilities introduced in this PR.
9-13: LGTM! Resolver injection refactored correctly.The plugin transformation logic now uses
utils.addResolverto inject the SkResolver into the autoImport plugin. This approach is consistent with other UI templates and improves maintainability by centralizing resolver injection logic.packages/core/template/UI/uview-pro/vite.config.js.data.mjs (3)
1-1: LGTM! Signature updated to support utilities.The function signature now accepts the
utilsparameter, enabling centralized resolver injection and deep config merging.
19-23: LGTM! Resolver injection refactored correctly.The plugin transformation uses
utils.addResolverto inject the uViewProResolver into the autoImport plugin, maintaining consistency with other UI templates.
24-24: LGTM! Deep merge applied correctly.The
extraConfigis now merged usingutils.deepMerge, which properly handles nested configuration objects and arrays. The fallback to an empty object ensures safe merging even whenoldData.extraConfigis undefined.packages/core/template/UI/nut/vite.config.js.data.mjs (3)
1-1: LGTM! Signature updated to support utilities.The function signature now accepts the
utilsparameter, enabling centralized utilities for resolver injection and config merging.
19-23: LGTM! Resolver injection refactored correctly.The plugin transformation uses
utils.addResolverto inject the NutResolver into the autoImport plugin, maintaining consistency across UI templates.
24-24: LGTM! Deep merge applied correctly.The
extraConfigmerging now usesutils.deepMerge, which properly handles nested objects and arrays. The fallback to an empty object ensures safe operation whenoldData.extraConfigis undefined.packages/core/template/UI/ano/vite.config.js.data.mjs (1)
1-15: LGTM. Implementation matches other UI templates and no extraConfig or SCSS setup is required for ano-ui. Manually verify that theAnoResolverexport exists in@uni-helper/vite-plugin-uni-components/resolvers.
| if (plugin.initializer == null || plugin.initializer.includes(resolver)) { | ||
| return plugin | ||
| } |
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.
Substring check may cause false positives.
The includes(resolver) check on line 5 uses substring matching, which could cause false positives. For example, if resolver = "Foo" and initializer contains "FooResolver", the function would incorrectly skip injection.
Consider using a more precise check, such as a regex word boundary or matching the exact resolver within the resolvers array.
- if (plugin.initializer == null || plugin.initializer.includes(resolver)) {
+ if (plugin.initializer == null || new RegExp(`\\b${resolver}\\b`).test(plugin.initializer)) {
return plugin
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (plugin.initializer == null || plugin.initializer.includes(resolver)) { | |
| return plugin | |
| } | |
| if (plugin.initializer == null || new RegExp(`\\b${resolver}\\b`).test(plugin.initializer)) { | |
| return plugin | |
| } |
🤖 Prompt for AI Agents
In packages/core/src/utils/injectUtils.ts around lines 5 to 7, the use of
plugin.initializer.includes(resolver) performs substring matching and can
produce false positives (e.g. "Foo" matching "FooResolver"); change the check to
a precise match by either (a) ensuring initializer is parsed as an array and
using a strict equality check (initializerArray.includes(resolver)) or (b) use a
regex word-boundary match (new
RegExp(`\\b${escapeForRegex(resolver)}\\b`).test(initializer)) so only exact
resolver names are considered; implement one of these approaches and add a small
helper escapeForRegex to safely build the regex if using option (b).
|
好厉害, export default function getData({ oldData, utils }) {
const promisesExtraConfig = {
build: {
target: 'es6',
cssTarget: 'chrome61',
},
optimizeDeps: {
exclude: ['vue-demi'],
},
}
return {
...oldData,
// 把空值判断放到extraMerge里
extraConfig: utils.extraMerge(oldData.extraConfig, promisesExtraConfig),
}
}export default function getData({ oldData, utils }) {
const autoImportWotDesignUiPlugin = {
id: 'wot-design-ui',
importer: `import { WotResolver } from '@uni-helper/vite-plugin-uni-components/resolvers'`,
}
return {
...oldData,
plugins: utils.autoImportPluginMerge(oldData, autoImportWotDesignUiPlugin)
}
}大概像上面这样 |
|
添加了 但是 export default function getData({ oldData, utils }) {
const uniEchartsPlugin = {
id: 'uni-echarts',
importer: `import { UniEcharts } from 'uni-echarts/vite'`,
initializer: `// https://uni-echarts.xiaohe.ink
UniEcharts()`,
}
const autoImportUniEchartsPlugin = {
id: 'uni-echarts-auto-import',
importer: `import { UniEchartsResolver } from 'uni-echarts/resolver'`,
}
const uniEchartsExtraConfig = {
optimizeDeps: {
exclude: ['uni-echarts'],
},
}
return {
...oldData,
plugins: oldData.plugins.flatMap((plugin) => {
if (plugin.id === 'uni') {
return [uniEchartsPlugin, plugin]
}
if (plugin.id === 'autoImport') {
return [utils.addResolver(plugin, 'UniEchartsResolver()'), autoImportUniEchartsPlugin]
}
return plugin
}),
extraConfig: utils.mergeExtraConfig(oldData.extraConfig, uniEchartsExtraConfig),
}
}我添加一个 |
Description 描述
此 PR 向
getData方法中注入了utils工具集,可以方便配置处理。除组件库之外,其他模块也有可能会提供 Components Resolver,所以新增了
addResolver工具方法用于添加额外的 Resolver;由于 extraConfig 可能需要深层次合并以及数组处理,所以添加了
mergeExtraConfig方法用于合并配置。Summary by CodeRabbit
New Features
Refactor
Style