fix:自定义渲染函数中的组件，如果页面没有引入，预览页面和出码页面会存在问题 #1146
Conversation
WalkthroughThe changes introduce a new hook, Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 ESLint
packages/vue-generator/src/generator/vue/sfc/generateScript.jsOops! Something went wrong! :( ESLint: 8.57.1 Error: Cannot read config file: /packages/vue-generator/.eslintrc.cjs
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
packages/vue-generator/src/generator/vue/sfc/generateScript.js (3)
130-136: Consider enhancing the component extraction regex pattern.The current regex pattern
/<[A-Z][a-zA-Z0-9]*/gmay miss certain component formats:
- Doesn't handle namespaced components like
<Namespace.Component>- Won't extract components from JSX attributes like
<div component={<AnotherComponent />} />extractComponents(jsContent) { - const matches = jsContent.match(/<[A-Z][a-zA-Z0-9]*/g) || [] + // Match standalone components and those within namespaces + const tagMatches = jsContent.match(/<([A-Z][a-zA-Z0-9]*(\.[A-Z][a-zA-Z0-9]*)*)/g) || [] + // Extract actual component names (handling both standalone and namespaced) + const componentNames = tagMatches.map(comp => { + const withoutBracket = comp.slice(1) + // For namespaced components, take the part after the last dot + return withoutBracket.includes('.') ? withoutBracket : withoutBracket + }) + return [...new Set(componentNames)] }
138-165: Add error handling to the JSX component processing function.The function lacks error handling which could lead to silent failures if an unexpected structure is encountered in the schema, particularly when processing JSX content.
export const handleJSXComponentsHook = (schema, hooks, config) => { const componentsMap = config?.componentsMap || [] const processJSXContent = (jsContent) => { + try { const components = componentNameUtils.extractComponents(jsContent) components.forEach((componentName) => { const componentConfig = componentsMap.find((cfg) => cfg.componentName === componentName) if (componentConfig) { const { package: pkgName, destructuring, componentName, exportName } = componentConfig hooks.addImport(pkgName, { destructuring, componentName, exportName }) } }) + } catch (error) { + console.warn('Error processing JSX content:', error) + } } const traverseSchema = (obj) => { if (!obj) return + try { Object.values(obj).forEach((value) => { if (value?.type === 'JSFunction') { processJSXContent(value.value) } else if (typeof value === 'object') { traverseSchema(value) } }) + } catch (error) { + console.warn('Error traversing schema:', error) + } } traverseSchema(schema) }
152-162: Optimize schema traversal for performance.The current recursive traversal could be inefficient for large schemas. Consider adding a depth limit or more targeted traversal.
- const traverseSchema = (obj) => { + const traverseSchema = (obj, depth = 0, maxDepth = 20) => { if (!obj) return + // Prevent excessive recursion for deep objects + if (depth > maxDepth) return Object.values(obj).forEach((value) => { if (value?.type === 'JSFunction') { processJSXContent(value.value) - } else if (typeof value === 'object') { + } else if (typeof value === 'object' && value !== null) { - traverseSchema(value) + traverseSchema(value, depth + 1, maxDepth) } }) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/vue-generator/src/generator/vue/sfc/genSetupSFC.js(2 hunks)packages/vue-generator/src/generator/vue/sfc/generateScript.js(1 hunks)
🔇 Additional comments (2)
packages/vue-generator/src/generator/vue/sfc/genSetupSFC.js (2)
40-41: Appropriate import added for the JSX components hook.The
handleJSXComponentsHookhas been properly imported from the generateScript module.
246-247: Integration of JSX components handling into the parse script pipeline.The
handleJSXComponentsHookhas been appropriately added to thedefaultParseScriptHookarray, which will enable automatic detection and import of components used in custom rendering functions.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/vue-generator/src/generator/vue/sfc/generateScript.js (1)
145-172: Robust JSX component processing implementation with good error handlingThe hook logic for processing JSX content and adding the necessary imports is well-structured. The error handling is comprehensive, providing detailed context for debugging.
One suggestion for improvement:
Consider reducing the verbosity of error messages in production environments to avoid exposing internal implementation details:
- throw new Error('Error processing JSX content:', error, '\nContent:', jsContent) + throw new Error(`Error processing JSX content: ${error.message}`)Similar changes could be applied to other error messages in this function.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/vue-generator/src/generator/vue/sfc/generateScript.js(1 hunks)
🔇 Additional comments (2)
packages/vue-generator/src/generator/vue/sfc/generateScript.js (2)
174-197: Well-implemented schema traversal with safety mechanismsThe recursive traversal implementation is sound with a depth limit to prevent infinite recursion. The function effectively processes JSFunction types and navigates through nested objects.
199-203: Robust top-level error handlingThe try-catch block at the top level ensures that any errors during schema traversal are properly caught and reported, which is essential for debugging issues in complex schemas.
| // 提取组件名称处理相关函数 | ||
| const componentNameUtils = { | ||
| extractComponents(jsContent) { | ||
| // Match standalone components and those within namespaces | ||
| const tagMatches = jsContent.match(/<([A-Z][a-zA-Z0-9]*(\.[A-Z][a-zA-Z0-9]*)*)/g) || [] | ||
| // Extract actual component names (handling both standalone and namespaced) | ||
| const componentNames = tagMatches.map((comp) => { | ||
| const withoutBracket = comp.slice(1) | ||
| // For namespaced components, take the part after the last dot | ||
| return withoutBracket.includes('.') ? withoutBracket : withoutBracket | ||
| }) | ||
| return [...new Set(componentNames)] | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Good implementation of JSX component extraction utility
The componentNameUtils object provides a clean way to extract component names from JSX content. The regex pattern correctly identifies components starting with capital letters, which follows JSX convention.
However, there's an issue with namespaced component handling.
Fix the namespaced component extraction logic:
- return withoutBracket.includes('.') ? withoutBracket : withoutBracket
+ // For namespaced components, take the whole name including namespace
+ return withoutBracketThe current ternary condition doesn't change the behavior based on whether there's a dot or not - both cases return withoutBracket.
📝 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.
| // 提取组件名称处理相关函数 | |
| const componentNameUtils = { | |
| extractComponents(jsContent) { | |
| // Match standalone components and those within namespaces | |
| const tagMatches = jsContent.match(/<([A-Z][a-zA-Z0-9]*(\.[A-Z][a-zA-Z0-9]*)*)/g) || [] | |
| // Extract actual component names (handling both standalone and namespaced) | |
| const componentNames = tagMatches.map((comp) => { | |
| const withoutBracket = comp.slice(1) | |
| // For namespaced components, take the part after the last dot | |
| return withoutBracket.includes('.') ? withoutBracket : withoutBracket | |
| }) | |
| return [...new Set(componentNames)] | |
| } | |
| } | |
| // 提取组件名称处理相关函数 | |
| const componentNameUtils = { | |
| extractComponents(jsContent) { | |
| // Match standalone components and those within namespaces | |
| const tagMatches = jsContent.match(/<([A-Z][a-zA-Z0-9]*(\.[A-Z][a-zA-Z0-9]*)*)/g) || [] | |
| // Extract actual component names (handling both standalone and namespaced) | |
| const componentNames = tagMatches.map((comp) => { | |
| const withoutBracket = comp.slice(1) | |
| // For namespaced components, take the whole name including namespace | |
| return withoutBracket | |
| }) | |
| return [...new Set(componentNames)] | |
| } | |
| } |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
packages/vue-generator/src/generator/vue/sfc/generateScript.js (3)
144-171: Effective component processing with robust error handlingThe handleJSXComponentsHook and processJSXContent functions correctly identify components in JSX content and add the necessary imports. The error handling is comprehensive with specific error messages.
However, the Error constructor only accepts a single message argument, so additional arguments like the error object will be ignored.
- throw new Error('Invalid JSX content:', jsContent) + throw new Error(`Invalid JSX content: ${jsContent}`)Similar fixes should be applied to other error messages in the code.
173-196: Improvement needed in schema traversal error handlingThe recursive traversal logic is well-implemented with a depth limit to prevent infinite recursion. However, the error message construction needs improvement.
The error message construction has two issues:
- The Error constructor only accepts a single message parameter
- The error condition check could be improved for clarity
- if (!obj || depth > MAX_DEPTH) { - throw new Error('Schema traversal stopped: ', depth > MAX_DEPTH ? 'Max depth exceeded' : 'Invalid object') + if (depth > MAX_DEPTH) { + throw new Error(`Schema traversal stopped: Max depth exceeded (${depth})`) + } + + if (!obj) { + throw new Error('Schema traversal stopped: Invalid object')Similar error handling improvements should be applied to the error at line 194.
198-203: Final error handling needs improvementThe top-level error handling is good for capturing any issues during schema traversal, but has the same issue with Error constructor arguments.
- throw new Error('Fatal error in handleJSXComponentsHook:', error) + throw new Error(`Fatal error in handleJSXComponentsHook: ${error.message || error}`)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/vue-generator/src/generator/vue/sfc/generateScript.js(1 hunks)
🔇 Additional comments (1)
packages/vue-generator/src/generator/vue/sfc/generateScript.js (1)
130-142: Good implementation of JSX component extraction utilityThe componentNameUtils object provides a clean way to extract component names from JSX content. The regex pattern correctly identifies components starting with capital letters, which follows JSX convention.
English | 简体中文
PR
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
Background and solution
自定义渲染函数中的组件，如果页面没有引入，预览页面和出码页面会存在问题
当前出码还不支持检测自定义渲染函数中使用到的组件并生成对应的 import 语句
在 generateScript 的时候 追加 handleJSXComponentsHook 获取自定义的渲染函数，从里面抽取组件名， 根据componentsMap的映射关系拿到package名称自动导入
What is the current behavior?
Issue Number: #1127
What is the new behavior?
自定义渲染函数中的组件，如果页面没有引入，预览页面和出码页面会自动导入依赖，正常显示
Does this PR introduce a breaking change?
Other information
Summary by CodeRabbit