-
-
Notifications
You must be signed in to change notification settings - Fork 9k
types(jsx): add new HTML attributes for improved JSX support #13370
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
- Move `enterKeyHint`, `exportparts`, and `part` attributes to `HTMLAttributes`. - Added `fetchpriority` attribute to `ImgHTMLAttributes`. - Updated `IntrinsicElementAttributes` to include `set` element support.
WalkthroughJSX type declarations in Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Potential attention points:
Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 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 |
Size ReportBundles
Usages
|
@vue/compiler-core
@vue/compiler-dom
@vue/compiler-sfc
@vue/compiler-ssr
@vue/reactivity
@vue/runtime-core
@vue/runtime-dom
@vue/server-renderer
@vue/shared
vue
@vue/compat
commit: |
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 (1)
packages/runtime-dom/src/jsx.ts (1)
362-369: Globalexportpartsandpartattributes are appropriately exposedAdding
exportpartsandpartas string-typed global HTML attributes matches their usage for web components and fits the existing typing approach in this interface.If you want stricter consistency with nearby attributes (e.g.
id?: string | undefined), you could also add| undefinedto both types, but the current form is functionally fine.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/runtime-dom/src/jsx.ts(4 hunks)
⏰ 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). (6)
- GitHub Check: test / lint-and-test-dts
- GitHub Check: test / e2e-test
- GitHub Check: test / unit-test
- GitHub Check: test / unit-test-windows
- GitHub Check: continuous-release
- GitHub Check: autofix
🔇 Additional comments (2)
packages/runtime-dom/src/jsx.ts (2)
289-301: Canonicalenterkeyhint+ deprecatedenterKeyHintalias look correctUsing lowercase
enterkeyhintas the primary attribute and a deprecatedenterKeyHintalias typed viaHTMLAttributes['enterkeyhint']is consistent with the rest of this file and avoids duplication while keeping backwards compatibility.
1304-1304: Addingsetto intrinsic SVG elements completes coverageIncluding
set: SVGAttributesalongside the other SVG animation elements (e.g.animate,animateTransform) is consistent with the existing mapping and improves JSX coverage for SVG.
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: 0
🧹 Nitpick comments (1)
packages/runtime-dom/src/jsx.ts (1)
362-369: Add| undefinedtoexportpartsandpartfor consistency.Both attributes should include
| undefinedto match the pattern used by all other optional string attributes inHTMLAttributes(e.g.,accesskey,dir,id,lang,title).Apply this diff:
/** * @see https://developer.mozilla.org/en-US/docs/Web/HTML/Global_attributes/exportparts */ - exportparts?: string + exportparts?: string | undefined /** * @see https://developer.mozilla.org/en-US/docs/Web/HTML/Global_attributes/part */ - part?: string + part?: string | undefined
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/runtime-dom/src/jsx.ts(4 hunks)
⏰ 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). (2)
- GitHub Check: test / e2e-test
- GitHub Check: test / unit-test-windows
🔇 Additional comments (3)
packages/runtime-dom/src/jsx.ts (3)
289-301: Excellent resolution of the casing issue!The lowercase
enterkeyhintas the primary attribute with a deprecated camelCaseenterKeyHintalias properly addresses the previous review concern. This follows the file's convention of using lowercase for HTML attributes while providing backward compatibility.
522-522: LGTM!The
fetchpriorityattribute correctly includes| undefinedand is consistent with other optional union-type attributes inImgHTMLAttributes.
1304-1304: Addition ofsetelement is correct.The
<set>element is one of the four standard SVG animation elements, and the mapping toSVGAttributesis appropriate. Verification confirms SVGAttributes is properly defined and exported at line 849, with no conflicts in the codebase.
|
This breaks Vant's CI: https://github.com/vuejs/ecosystem-ci/actions/runs/19490366056/job/55781263942 |
enterKeyHint,exportparts, andpartattributes toHTMLAttributes.According to [@types/react-dom] enterKeyHint not defined on most elements DefinitelyTyped/DefinitelyTyped#70409
fetchpriorityattribute toImgHTMLAttributes.IntrinsicElementAttributesto includesetelement support.Summary by CodeRabbit
New Features
Deprecated / Removed