Repository navigation
feat: 支持自定义组件props类型提示支持 - #108
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds runtime prop type utilities, richer component self-typing, runtime ChangesRuntime Prop Types and JSON usingComponents Support
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant MPXPlugin
participant ScriptCodegen
participant JSONUsingComponents
participant TypeScriptPlugin
participant TemplateLanguageService
MPXPlugin->>ScriptCodegen: pass component names and self component name
ScriptCodegen->>JSONUsingComponents: generate imports and runtime mappings
JSONUsingComponents-->>ScriptCodegen: emit __MPX_jsonComponents
TemplateLanguageService->>TypeScriptPlugin: request component prop metadata
TypeScriptPlugin-->>TemplateLanguageService: return merged prop types
TemplateLanguageService-->>MPXPlugin: provide typed template completion and hover
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
packages/language-core/src/codegen/script/index.ts (1)
35-35: ⚡ Quick winAdd JSDoc for the new required field.
The
scriptSetupImportComponentNamesfield is added without documentation. Consider adding a JSDoc comment explaining what component names are included (e.g., "Set of component names imported via default import from .mpx files in script setup").📝 Proposed documentation
scriptSetupRanges: ScriptSetupRanges | undefined + /** + * Set of component names imported via default import from .mpx files in <script setup>. + * Used for resolving component types in template code generation. + */ scriptSetupImportComponentNames: Set<string> templateCodegen: (TemplateCodegenContext & { codes: Code[] }) | undefined🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/language-core/src/codegen/script/index.ts` at line 35, Add a JSDoc comment above the scriptSetupImportComponentNames field describing its purpose and contents: state that it is a Set<string> containing component names that are imported via default import from .mpx files inside a script setup block (i.e., components brought into scope by `import X from "Y.mpx"` in script setup), note that it is required and used to track which components should be treated as locally registered in generated code, and include a short example usage to clarify semantics.packages/language-core/src/codegen/script/jsonUsingComponents.ts (1)
112-122: ⚡ Quick winConsider adding index suffix for consistency.
The function returns just
${baseName}without any index suffix (line 121). When multiple paths resolve to the same component name, they'll have different indices, but the first one (index 0) won't have a distinguishing suffix. Consider returning${baseName}_${index}or${baseName}${index === 0 ? '' :_${index}}for consistency and to avoid potential name collisions if the same component name appears multiple times.🔧 Proposed refinement for consistency
export function getUsingComponentImportName( componentName: string, index: number, ) { const camelizedName = capitalize(camelize(componentName)) const baseName = camelizedName && identifierRegex.test(camelizedName) ? camelizedName : `component_${index}` - return `${baseName}` + return index > 0 && camelizedName && identifierRegex.test(camelizedName) + ? `${baseName}_${index}` + : baseName }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/language-core/src/codegen/script/jsonUsingComponents.ts` around lines 112 - 122, getUsingComponentImportName currently returns baseName without an index suffix which can collide when the same componentName appears multiple times; update the function (referencing getUsingComponentImportName, camelizedName, identifierRegex, baseName) to append a deterministic index suffix such as `${baseName}_${index}` (or `${baseName}${index === 0 ? '' : `_${index}`}` if you want to keep the original for index 0) so each generated import name is unique and consistent across duplicates.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@inspect-extension/components/reference-type-props/component-options.mpx`:
- Around line 17-21: The JSON script block uses the incorrect symbol
"module.export"; update it to the standard Node.js export symbol
"module.exports" in the JSON script (the object assigned to module.exports) so
the export matches the rest of the codebase and other JSON blocks—locate the
script named "json" in component-options.mpx and replace module.export with
module.exports.
In `@inspect-extension/components/reference-type-props/index.mpx`:
- Line 3: This line intentionally uses a numeric literal for the msg prop of the
component-js-setup component to trigger type-checking; add an inline comment
next to the component invocation (component-js-setup msg="{{ 123 }}") stating
that this is test code in reference-type-props and that the number is an
intentional type mismatch for verification of the string-typed msg prop defined
in component-js-setup.mpx.
- Line 2: The parentText prop on the component-options component is receiving a
number ({{ 123 }}) while component-options.mpx defines parentText as type
String; either change the value to a string literal to match the declared type
(e.g., provide a string expression) or add an inline comment near the component
usage (or above the file) stating that the numeric value is intentionally used
to test type-checking; reference the component-options component and its
parentText prop declared in component-options.mpx when making the change so the
mismatch is resolved or explicitly documented.
---
Nitpick comments:
In `@packages/language-core/src/codegen/script/index.ts`:
- Line 35: Add a JSDoc comment above the scriptSetupImportComponentNames field
describing its purpose and contents: state that it is a Set<string> containing
component names that are imported via default import from .mpx files inside a
script setup block (i.e., components brought into scope by `import X from
"Y.mpx"` in script setup), note that it is required and used to track which
components should be treated as locally registered in generated code, and
include a short example usage to clarify semantics.
In `@packages/language-core/src/codegen/script/jsonUsingComponents.ts`:
- Around line 112-122: getUsingComponentImportName currently returns baseName
without an index suffix which can collide when the same componentName appears
multiple times; update the function (referencing getUsingComponentImportName,
camelizedName, identifierRegex, baseName) to append a deterministic index suffix
such as `${baseName}_${index}` (or `${baseName}${index === 0 ? '' :
`_${index}`}` if you want to keep the original for index 0) so each generated
import name is unique and consistent across duplicates.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 121ec58c-f129-469c-bdde-f34c59d885f7
📒 Files selected for processing (14)
inspect-extension/components/reference-type-props/component-js-setup.mpxinspect-extension/components/reference-type-props/component-options.mpxinspect-extension/components/reference-type-props/index.mpxinspect-extension/components/script-json/components/list.mpxpackages/language-core/src/codegen/globalTypes/defineComponentTypes.tspackages/language-core/src/codegen/globalTypes/index.tspackages/language-core/src/codegen/script/componentSelf.tspackages/language-core/src/codegen/script/index.tspackages/language-core/src/codegen/script/jsonUsingComponents.tspackages/language-core/src/codegen/script/template.tspackages/language-core/src/codegen/template/element.tspackages/language-core/src/codegen/template/elementProps.tspackages/language-core/src/codegen/template/templateChild.tspackages/language-core/src/plugins/mpx-tsx.ts
💤 Files with no reviewable changes (1)
- packages/language-core/src/codegen/template/templateChild.ts
- infer and merge props from multiple component candidates - add prop completion, JSDoc hover, and nested object completion - avoid duplicate TypeScript and HTML completion/hover entries - prevent template codegen hangs while editing unmatched component tags
closes #86