fix(types): narrow useAttrs class/style typing for TSX#14492
fix(types): narrow useAttrs class/style typing for TSX#14492edison1105 merged 2 commits intomainfrom
Conversation
📝 WalkthroughWalkthroughThis PR fixes a TypeScript regression by introducing a new extendable Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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.
🧹 Nitpick comments (1)
packages-private/dts-test/tsx.test-d.tsx (1)
64-67: Add a negative assertion to prevent over-wideningclasstyping.Please pair this with a
//@ts-expect-error`` case (e.g.class={123}) so the suite also enforces that invalid class values remain rejected.Suggested dts-test addition
// allow class passthrough from attrs const attrs = useAttrs() expectType<JSX.Element>(<div class={attrs.class} />) + +// `@ts-expect-error` invalid class value should still be rejected +;<div class={123} />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages-private/dts-test/tsx.test-d.tsx` around lines 64 - 67, Add a negative type assertion to the existing test that verifies class passthrough from useAttrs does not over-widen the type: after the positive expectType<JSX.Element>(<div class={attrs.class} />) add a failing case annotated with // `@ts-expect-error` that tries an invalid class value (for example class={123}) to ensure the compiler still rejects non-string/non-string[] class values; update the tsx.test-d.tsx around the useAttrs/attrs assertions so both the valid passthrough and the explicit invalid-case are present.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages-private/dts-test/tsx.test-d.tsx`:
- Around line 64-67: Add a negative type assertion to the existing test that
verifies class passthrough from useAttrs does not over-widen the type: after the
positive expectType<JSX.Element>(<div class={attrs.class} />) add a failing case
annotated with // `@ts-expect-error` that tries an invalid class value (for
example class={123}) to ensure the compiler still rejects
non-string/non-string[] class values; update the tsx.test-d.tsx around the
useAttrs/attrs assertions so both the valid passthrough and the explicit
invalid-case are present.
|
LGTM But I'd like to expand the type Data = Record<string, unknown>;See: core/packages/runtime-core/src/component.ts Line 286 in cea3cf7 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/runtime-core/src/component.ts`:
- Around line 101-104: Add an inline Biome rule ignore immediately above the
export of the empty AllowedAttrs interface to suppress the noEmptyInterface
rule; keep the exported declaration export interface AllowedAttrs {} as the
public extension point and place a single-line or block ignore (e.g. /*
biome-ignore noEmptyInterface */) directly above that declaration so the
intentional empty interface used for declaration-merging is not flagged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7427b117-a9d7-4e7f-8e90-560e76c472eb
📒 Files selected for processing (5)
packages-private/dts-test/tsx.test-d.tsxpackages/runtime-core/src/component.tspackages/runtime-core/src/componentPublicInstance.tspackages/runtime-core/src/index.tspackages/runtime-dom/src/index.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages-private/dts-test/tsx.test-d.tsx
|
/ecosystem-ci run |
|
📝 Ran ecosystem CI: Open
|
close #14489
Summary by CodeRabbit
Release Notes
New Features
AttrsandAllowedAttrstype definitions.Tests