[pull] master from vuejs:master #140
Conversation
![pull[bot]](https://avatars.githubusercontent.com/in/12910?s=60&v=4){kind=small}
Co-authored-by: A5rocks <[email protected]> Co-authored-by: Johnson Chu <[email protected]>
…4784) * refactor(language-core): wrap template virtual code into a function * fix: drop `declare` --------- Co-authored-by: KazariEX <[email protected]>
![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
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
packages/tsc/tests/__snapshots__/dts.spec.ts.snapis excluded by!**/*.snap
Files selected for processing (5)
- packages/language-core/lib/codegen/script/component.ts (3 hunks)
- packages/language-core/lib/codegen/script/index.ts (4 hunks)
- packages/language-core/lib/codegen/script/scriptSetup.ts (11 hunks)
- packages/language-core/lib/codegen/script/template.ts (5 hunks)
- packages/language-core/lib/codegen/template/index.ts (3 hunks)
Files skipped from review as they are similar to previous changes (1)
- packages/language-core/lib/codegen/script/index.ts
Additional comments not posted (23)
packages/language-core/lib/codegen/template/index.ts (6)
8-8: LGTM!The code changes are approved.
20-24: LGTM!The code changes are approved. The addition of
templateRefNamesandinheritAttrsproperties to theTemplateCodegenOptionsinterface enhances the flexibility of the template code generation process, as mentioned in the PR summary.
28-28: LGTM!The code change is approved. Passing the entire
optionsobject tocreateTemplateCodegenContextinstead of justoptions.scriptSetupBindingNamesreflects a change in how context is managed during the generation process, as mentioned in the PR summary.
52-72: LGTM! But address the previous review comment.The code changes are approved. The introduction of the
generateRefsfunction enhances the handling of template references, as mentioned in the PR summary.However, the previous review comment is still valid and applicable:
Implement error handling in
generateRefs.The function
generateRefscorrectly iterates overtemplateRefNamesto generate references. However, as previously suggested, adding error handling for empty or invalid entries would enhance robustness. This is especially important given the dynamic nature of template references in Vue applications.Consider adding the following error handling:
if (!options.templateRefNames || options.templateRefNames.size === 0) { throw new Error('No template references provided.'); }Please address this comment by implementing the suggested error handling in
generateRefs.
105-110: LGTM! But address the previous review comment.The code changes are approved. The introduction of the
generateInheritedAttrsfunction enhances the management of inherited attributes, as mentioned in the PR summary.However, the previous review comment is still valid and applicable:
Implement error handling in
generateInheritedAttrs.The function
generateInheritedAttrsconstructs the variable__VLS_inheritedAttrsand iterates overinheritedAttrVars. Similar togenerateRefs, adding error handling for empty or invalid entries ininheritedAttrVarswould prevent runtime errors and ensure that the function behaves as expected under all conditions.Consider adding the following error handling:
if (!ctx.inheritedAttrVars || ctx.inheritedAttrVars.size === 0) { throw new Error('No inherited attributes provided.'); }Please address this comment by implementing the suggested error handling in
generateInheritedAttrs.
Line range hint
114-136: LGTM!The code changes are approved. The modifications to the
generatePreResolveComponentsfunction correctly generate code for resolving local and global components by iterating over the template AST and identifying component nodes.packages/language-core/lib/codegen/script/component.ts (2)
39-41: LGTM!The code changes are approved.
43-45: LGTM!The code changes are approved.
packages/language-core/lib/codegen/script/template.ts (7)
7-7: LGTM!The code changes are approved.
11-44: Approve the renaming and parameter changes ingenerateTemplateCtx.The changes to the function name and parameters reflect a clearer focus and potentially simplify the interface. The generated context variable is now more structured, incorporating various types based on the component type and the presence of styles.
91-118: Review the integration of functions ingenerateTemplate.The function
generateTemplateeffectively integrates new and modified functions, orchestrating the template generation process seamlessly. Comprehensive integration testing is recommended to ensure that all components work together without issues, especially given the significant changes in function responsibilities and data flow.Consider setting up comprehensive integration tests to validate the entire template generation process.
Line range hint
120-173: Approve the changes ingenerateTemplateBody.The renaming of the function to
generateTemplateBodyindicates a clearer separation of concerns in the template generation workflow. The function now yields a structured template result object, improving the overall output format and usability within Vue components. The introduction of thegenerateStyleScopedClassesfunction call signifies a new mechanism for handling style-scoped classes, indicating a more sophisticated approach to CSS class generation based on the styles defined in the component.
Line range hint
175-203: LGTM!The code changes are approved.
Line range hint
205-238: LGTM!The code changes are approved.
46-88: Approve the introduction ofgenerateTemplateComponents.This new function enhances modularity by isolating component-related logic, which should improve maintainability and clarity. It is recommended to add unit tests for this function to ensure it handles various scenarios correctly.
Would you like assistance in creating unit tests for this function?
packages/language-core/lib/codegen/script/scriptSetup.ts (8)
2-2: LGTM!The code change to import the
TextRangetype is approved.
7-7: LGTM!The code change to import the
generateInternalComponentfunction is approved.
8-8: LGTM!The code change to import the
generateCssClassPropertyandgenerateTemplatefunctions is approved.
56-56: LGTM!The code change to use
ctx.localTypes.PrettifyLocalinstead of__VLS_Prettifyis approved. The comment indicates that this change is made to generate less dts code.
61-67: LGTM!The code changes to the logic for handling emitted types are approved. The changes reflect a change in how emitted types are constructed based on the presence of defined properties. The change from unshifting to pushing to the
emitTypesarray is a minor refactor that does not introduce any issues.
71-75: LGTM!The code changes to the returned object are approved. The changes reflect a shift towards a more type-safe and structured approach to generating script setups. The use of
ctx.localTypes.PrettifyLocaland__VLS_TemplateResult['slots']enhances type safety and clarity. The changes to theslotsandemitproperties align with modern TypeScript practices and do not introduce any issues.
Line range hint
95-109: LGTM!The code changes to handle
definePropproperties are approved. The introduction of thegetPropAndLocalNamefunction centralizes the logic for retrieving property names and local names, streamlining the code and reducing redundancy. This change enhances maintainability and readability. The generation of linked code mappings for properties with a local name is a new feature that does not introduce any issues.
Line range hint
218-550: LGTM!The code changes introduce several new features and refactor existing code to enhance maintainability, readability, and type safety. The introduction of new functions such as
generateStyleModules,generateInternalComponent,getPropAndLocalName, andgetRangeNamecentralizes logic and reduces redundancy. The use ofctx.localTypesinstead of__VLS_prefixed types aligns with modern TypeScript practices and enhances type safety. The new logic for handling CSS modules, template refs, and model emits expands the capabilities of the script setup generation. The changes do not introduce any issues.
johnsoncodehk
force-pushed
the
master
branch
from
83562e6
to
bdf5d41
Compare
…s not enabled close #4785
johnsoncodehk
force-pushed
the
master
branch
from
36af845
to
1eabe25
Compare
…mponent template (#4796)
* feat: typed directives in template * fix: compatibility * format * Update main.vue * Update globalTypes.ts * fixes --------- Co-authored-by: Johnson Chu <[email protected]>
…emplate (#4809) * fix(language-core): prevent type error when use defineSlots and no template * fix: typo
Co-authored-by: 山吹色御守 <[email protected]>
See Commits and Changes for more details.
Created by
pull[bot]
Can you help keep this open source service alive? 💖 Please sponsor : )
Summary by CodeRabbit
New Features
template-ref.Bug Fixes
globalTypesand improved default behavior forvueCompilerOptions.lib.Tests
Chores