Skip to content
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.

Sign up for GitHub

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

types(runtime-dom): type for class property #8012

Merged
merged 18 commits into from
Nov 10, 2023
Merged

types(runtime-dom): type for class property #8012

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
05886c3
types: add type for 'class' prop of 'HTMLAttributes' type
Apr 3, 2023
86081a2
test: add more tests for 'normalizeClass'
Apr 3, 2023
e4e56ac
test(dts-test): add tests for element's 'class' attribute usage
Apr 3, 2023
2f81d54
types: allow truthy/falsy values in `ClassValue` type
Apr 14, 2023
2ba87e3
test: add tests for truthy/falsy values in `ClassValue` type
Apr 14, 2023
b7cec7c
Merge branch 'main' into type-for-html-attributes-class-prop
basil-gor Jun 19, 2023
6be1bd0
Merge branch 'main' into type-for-html-attributes-class-prop
basil-gor Jul 7, 2023
080e487
Merge branch 'main' into type-for-html-attributes-class-prop
basil-gor Jul 18, 2023
b7d6dbd
Merge branch 'main' into type-for-html-attributes-class-prop
basil-gor Aug 1, 2023
30da39f
types: allow numbers as keys in ClassValue type
basil-gor Aug 16, 2023
6a856fe
Merge branch 'main' into type-for-html-attributes-class-prop
basil-gor Aug 16, 2023
5139781
Merge branch 'main' into type-for-html-attributes-class-prop
basil-gor Aug 31, 2023
b1de9cb
Merge branch 'main' into type-for-html-attributes-class-prop
basil-gor Sep 13, 2023
d53bf59
Merge branch 'main' into type-for-html-attributes-class-prop
basil-gor Oct 3, 2023
6319fbd
Merge branch 'main' into type-for-html-attributes-class-prop
basil-gor Oct 20, 2023
2e03b76
Merge branch 'main' into type-for-html-attributes-class-prop
basil-gor Nov 8, 2023
c22fe2b
Merge branch 'main' into type-for-html-attributes-class-prop
yyx990803 Nov 10, 2023
9827bbb
[autofix.ci] apply automated fixes
autofix-ci[bot] Nov 10, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions packages/dts-test/tsx.test-d.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,18 @@ expectType<JSX.Element>(
<div style={[{ color: 'red' }, [{ fontSize: '1em' }]]} />
)

// allow undefined, string, object, array and nested array classes
expectType<JSX.Element>(<div class={undefined} />)
expectType<JSX.Element>(<div class={'foo'} />)
expectType<JSX.Element>(<div class={['foo', undefined, 'bar']} />)
expectType<JSX.Element>(<div class={[]} />)
expectType<JSX.Element>(<div class={['foo', ['bar'], [['baz']]]} />)
expectType<JSX.Element>(<div class={{ foo: true, bar: false, baz: true }} />)
expectType<JSX.Element>(<div class={{}} />)
expectType<JSX.Element>(
<div class={['foo', ['bar'], { baz: true }, [{ qux: true }]]} />
)

// @ts-expect-error unknown prop
;<div foo="bar" />

Expand Down
8 changes: 7 additions & 1 deletion packages/runtime-dom/src/jsx.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -236,10 +236,16 @@ interface AriaAttributes {
// Vue's style normalization supports nested arrays
export type StyleValue = string | CSSProperties | Array<StyleValue>

export type ClassValue =
| undefined
| string
| Record<string, boolean>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Technically the Record value can be any truthy/falsy value.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I understand correctly, you want me to extend the possible accepted values in the Record type to also accept truthy/falsy types for maximum backward compatibility with older versions of Vue?

For example:

type Truthy = true | string | number | {} | any[];
type Falsy = false | 0 | -0 | 0n | '' | null | undefined | NaN;

and result record type will be:

Record<string, Truthy | Falsy>

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not really, what I mean is the value should technically allow any since any value in JS is either truthy or falsy.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah. I understand what you mean.
So, then there are two possible solutions:

  1. Try to formalize it something like this:
type Falsy = false | 0 | -0 | 0n | '' | null | undefined | typeof NaN;
type Truthy<T> = T extends Falsy ? never : T;

export type ClassValue =
  | undefined
  | string
  | Record<string, Falsy | Truthy<unknown>>
  | Array<ClassValue>
  1. Just set the unknown type (or any at the very least):
export type ClassValue =
  | undefined
  | string
  | Record<string, unknown>
  | Array<ClassValue>

I think the second option will be easier and more priority, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@yyx990803 I'm sorry to bother you, but could you please rerewiew the changes? It's been two months since the last activity here. Thanks!

| Array<ClassValue>

export interface HTMLAttributes extends AriaAttributes, EventHandlers<Events> {
innerHTML?: string

class?: any
class?: ClassValue
style?: StyleValue

// Standard HTML Attributes
Expand Down
22 changes: 22 additions & 0 deletions packages/shared/__tests__/normalizeProp.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
import { normalizeClass, parseStringStyle } from '../src'

describe('normalizeClass', () => {
test('handles undefined correctly', () => {
expect(normalizeClass(undefined)).toEqual('')
})

test('handles string correctly', () => {
expect(normalizeClass('foo')).toEqual('foo')
})
Expand All @@ -11,12 +15,30 @@ describe('normalizeClass', () => {
)
})

test('handles empty array correctly', () => {
expect(normalizeClass([])).toEqual('')
})

test('handles nested array correctly', () => {
expect(normalizeClass(['foo', ['bar'], [['baz']]])).toEqual('foo bar baz')
})

test('handles object correctly', () => {
expect(normalizeClass({ foo: true, bar: false, baz: true })).toEqual(
'foo baz'
)
})

test('handles empty object correctly', () => {
expect(normalizeClass({})).toEqual('')
})

test('handles arrays and objects correctly', () => {
expect(
normalizeClass(['foo', ['bar'], { baz: true }, [{ qux: true }]])
).toEqual('foo bar baz qux')
})

// #6777
test('parse multi-line inline style', () => {
expect(
Expand Down