From 4a3c185e950c2d2785dc57b72a7c22dba84032bc Mon Sep 17 00:00:00 2001 From: Ubugeeei Date: Sat, 3 Feb 2024 19:08:25 +0900 Subject: [PATCH 1/4] feat(runtime-vapor): component props validator --- .../__tests__/componentProps_.spec.ts | 57 +++++++++ packages/runtime-vapor/src/componentProps.ts | 116 +++++++++++++++++- 2 files changed, 171 insertions(+), 2 deletions(-) create mode 100644 packages/runtime-vapor/__tests__/componentProps_.spec.ts diff --git a/packages/runtime-vapor/__tests__/componentProps_.spec.ts b/packages/runtime-vapor/__tests__/componentProps_.spec.ts new file mode 100644 index 000000000..f1845a545 --- /dev/null +++ b/packages/runtime-vapor/__tests__/componentProps_.spec.ts @@ -0,0 +1,57 @@ +// NOTE: merge into `componentProps.spec.ts` (https://github.com/vuejs/core-vapor/pull/99) + +import { defineComponent, render, template } from '../src' + +let host: HTMLElement +const initHost = () => { + host = document.createElement('div') + host.setAttribute('id', 'host') + document.body.appendChild(host) +} +beforeEach(() => initHost()) +afterEach(() => host.remove()) + +describe('component props (vapor)', () => { + test('validator', () => { + let args: any + const mockFn = vi.fn((..._args: any[]) => { + args = _args + return true + }) + + const Comp = defineComponent({ + props: { + foo: { + type: Number, + validator: (value: any, props: any) => mockFn(value, props), + }, + bar: { + type: Number, + }, + }, + render() { + const t0 = template('
') + const n0 = t0() + return n0 + }, + }) + + const props = { + get foo() { + return 1 + }, + get bar() { + return 2 + }, + } + + render(Comp, props, host) + expect(mockFn).toHaveBeenCalled() + // NOTE: Vapor Component props defined by getter. So, `props` not Equal to `{ foo: 1, bar: 2 }` + // expect(mockFn).toHaveBeenCalledWith(1, { foo: 1, bar: 2 }) + expect(args.length).toBe(2) + expect(args[0]).toBe(1) + expect(args[1].foo).toEqual(1) + expect(args[1].bar).toEqual(2) + }) +}) diff --git a/packages/runtime-vapor/src/componentProps.ts b/packages/runtime-vapor/src/componentProps.ts index 7b77f55bc..2593144cd 100644 --- a/packages/runtime-vapor/src/componentProps.ts +++ b/packages/runtime-vapor/src/componentProps.ts @@ -10,9 +10,11 @@ import { hyphenate, isArray, isFunction, + isObject, isReservedProp, + makeMap, } from '@vue/shared' -import { shallowReactive, toRaw } from '@vue/reactivity' +import { shallowReactive, shallowReadonly, toRaw } from '@vue/reactivity' import type { Component, ComponentInternalInstance } from './component' export type ComponentPropsOptions

= @@ -31,7 +33,7 @@ export interface PropOptions { type?: PropType | true | null required?: boolean default?: D | DefaultFactory | null | undefined | object - validator?(value: unknown): boolean + validator?(value: unknown, props: Data): boolean /** * @internal */ @@ -138,6 +140,11 @@ export function initProps( } } + // validation + if (__DEV__) { + validateProps(rawProps || {}, props, instance) + } + instance.props = shallowReactive(props) } @@ -265,3 +272,108 @@ function getTypeIndex( } return -1 } + +/** + * dev only + */ +function validateProps( + rawProps: Data, + props: Data, + instance: ComponentInternalInstance, +) { + const resolvedValues = toRaw(props) + const options = instance.propsOptions[0] + for (const key in options) { + let opt = options[key] + if (opt == null) continue + validateProp( + key, + resolvedValues[key], + opt, + __DEV__ ? shallowReadonly(resolvedValues) : resolvedValues, + !hasOwn(rawProps, key) && !hasOwn(rawProps, hyphenate(key)), + ) + } +} + +/** + * dev only + */ +function validateProp( + name: string, + value: unknown, + prop: PropOptions, + props: Data, + isAbsent: boolean, +) { + const { type, required, validator } = prop + // required! + if (required && isAbsent) { + // TODO: warn + // warn('Missing required prop: "' + name + '"') + return + } + // missing but optional + if (value == null && !required) { + return + } + // type check + if (type != null && type !== true) { + let isValid = false + const types = isArray(type) ? type : [type] + const expectedTypes = [] + // value is valid as long as one of the specified types match + for (let i = 0; i < types.length && !isValid; i++) { + const { valid, expectedType } = assertType(value, types[i]) + expectedTypes.push(expectedType || '') + isValid = valid + } + if (!isValid) { + // TODO: warn + // warn(getInvalidTypeMessage(name, value, expectedTypes)) + return + } + } + // custom validator + if (validator && !validator(value, props)) { + // TODO: warn + // warn('Invalid prop: custom validator check failed for prop "' + name + '".') + } +} + +const isSimpleType = /*#__PURE__*/ makeMap( + 'String,Number,Boolean,Function,Symbol,BigInt', +) + +type AssertionResult = { + valid: boolean + expectedType: string +} + +/** + * dev only + */ +function assertType(value: unknown, type: PropConstructor): AssertionResult { + let valid + const expectedType = getType(type) + if (isSimpleType(expectedType)) { + const t = typeof value + valid = t === expectedType.toLowerCase() + // for primitive wrapper objects + if (!valid && t === 'object') { + valid = value instanceof type + } + } else if (expectedType === 'Object') { + valid = isObject(value) + } else if (expectedType === 'Array') { + valid = isArray(value) + } else if (expectedType === 'null') { + valid = value === null + } else { + valid = value instanceof type + } + return { + valid, + expectedType, + } +} From 1cdd42fc777334e713efa8872cdd3b0bcc81cb26 Mon Sep 17 00:00:00 2001 From: Ubugeeei Date: Sat, 3 Feb 2024 19:29:09 +0900 Subject: [PATCH 2/4] feat(runtime-vapor): component props warn and remove typecheck --- .../__tests__/componentProps_.spec.ts | 86 ++++++++++++++++++- packages/runtime-vapor/src/componentProps.ts | 83 ++++++------------ 2 files changed, 108 insertions(+), 61 deletions(-) diff --git a/packages/runtime-vapor/__tests__/componentProps_.spec.ts b/packages/runtime-vapor/__tests__/componentProps_.spec.ts index f1845a545..d33b93ea1 100644 --- a/packages/runtime-vapor/__tests__/componentProps_.spec.ts +++ b/packages/runtime-vapor/__tests__/componentProps_.spec.ts @@ -11,8 +11,8 @@ const initHost = () => { beforeEach(() => initHost()) afterEach(() => host.remove()) -describe('component props (vapor)', () => { - test('validator', () => { +describe('validator', () => { + test('validator should be called with two arguments', () => { let args: any const mockFn = vi.fn((..._args: any[]) => { args = _args @@ -54,4 +54,86 @@ describe('component props (vapor)', () => { expect(args[1].foo).toEqual(1) expect(args[1].bar).toEqual(2) }) + + // TODO: impl setter and warnner + test.todo('validator should not be able to mutate other props', async () => { + const mockFn = vi.fn((...args: any[]) => true) + const Comp = defineComponent({ + props: { + foo: { + type: Number, + validator: (value: any, props: any) => !!(props.bar = 1), + }, + bar: { + type: Number, + validator: (value: any) => mockFn(value), + }, + }, + render() { + const t0 = template('

') + const n0 = t0() + return n0 + }, + }) + + render( + Comp, + { + get foo() { + return 1 + }, + get bar() { + return 2 + }, + }, + host, + ) + expect( + `Set operation on key "bar" failed: target is readonly.`, + ).toHaveBeenWarnedLast() + expect(mockFn).toHaveBeenCalledWith(2) + }) + + test('warn absent required props', () => { + const Comp = defineComponent({ + props: { + bool: { type: Boolean, required: true }, + str: { type: String, required: true }, + num: { type: Number, required: true }, + }, + setup() { + return () => null + }, + }) + render(Comp, {}, host) + expect(`Missing required prop: "bool"`).toHaveBeenWarned() + expect(`Missing required prop: "str"`).toHaveBeenWarned() + expect(`Missing required prop: "num"`).toHaveBeenWarned() + }) + + // NOTE: type check is not supported in vapor + // test('warn on type mismatch', () => {}) + + // #3495 + test('should not warn required props using kebab-case', async () => { + const Comp = defineComponent({ + props: { + fooBar: { type: String, required: true }, + }, + setup() { + return () => null + }, + }) + + render( + Comp, + { + get ['foo-bar']() { + return 'hello' + }, + }, + host, + ) + expect(`Missing required prop: "fooBar"`).not.toHaveBeenWarned() + }) }) diff --git a/packages/runtime-vapor/src/componentProps.ts b/packages/runtime-vapor/src/componentProps.ts index 2593144cd..634f38796 100644 --- a/packages/runtime-vapor/src/componentProps.ts +++ b/packages/runtime-vapor/src/componentProps.ts @@ -5,6 +5,7 @@ import { EMPTY_ARR, EMPTY_OBJ, camelize, + capitalize, extend, hasOwn, hyphenate, @@ -13,9 +14,11 @@ import { isObject, isReservedProp, makeMap, + toRawType, } from '@vue/shared' import { shallowReactive, shallowReadonly, toRaw } from '@vue/reactivity' import type { Component, ComponentInternalInstance } from './component' +import { warn } from './warning' export type ComponentPropsOptions

= | ComponentObjectPropsOptions

@@ -306,74 +309,36 @@ function validateProp( props: Data, isAbsent: boolean, ) { - const { type, required, validator } = prop + const { required, validator } = prop // required! if (required && isAbsent) { - // TODO: warn - // warn('Missing required prop: "' + name + '"') + warn('Missing required prop: "' + name + '"') return } // missing but optional if (value == null && !required) { return } - // type check - if (type != null && type !== true) { - let isValid = false - const types = isArray(type) ? type : [type] - const expectedTypes = [] - // value is valid as long as one of the specified types match - for (let i = 0; i < types.length && !isValid; i++) { - const { valid, expectedType } = assertType(value, types[i]) - expectedTypes.push(expectedType || '') - isValid = valid - } - if (!isValid) { - // TODO: warn - // warn(getInvalidTypeMessage(name, value, expectedTypes)) - return - } - } + // NOTE: type check is not supported in vapor + // // type check + // if (type != null && type !== true) { + // let isValid = false + // const types = isArray(type) ? type : [type] + // const expectedTypes = [] + // // value is valid as long as one of the specified types match + // for (let i = 0; i < types.length && !isValid; i++) { + // const { valid, expectedType } = assertType(value, types[i]) + // expectedTypes.push(expectedType || '') + // isValid = valid + // } + // if (!isValid) { + // warn(getInvalidTypeMessage(name, value, expectedTypes)) + // return + // } + // } + // custom validator if (validator && !validator(value, props)) { - // TODO: warn - // warn('Invalid prop: custom validator check failed for prop "' + name + '".') - } -} - -const isSimpleType = /*#__PURE__*/ makeMap( - 'String,Number,Boolean,Function,Symbol,BigInt', -) - -type AssertionResult = { - valid: boolean - expectedType: string -} - -/** - * dev only - */ -function assertType(value: unknown, type: PropConstructor): AssertionResult { - let valid - const expectedType = getType(type) - if (isSimpleType(expectedType)) { - const t = typeof value - valid = t === expectedType.toLowerCase() - // for primitive wrapper objects - if (!valid && t === 'object') { - valid = value instanceof type - } - } else if (expectedType === 'Object') { - valid = isObject(value) - } else if (expectedType === 'Array') { - valid = isArray(value) - } else if (expectedType === 'null') { - valid = value === null - } else { - valid = value instanceof type - } - return { - valid, - expectedType, + warn('Invalid prop: custom validator check failed for prop "' + name + '".') } } From 770274aefbb44d8c48e8bb00a505c4be1787a886 Mon Sep 17 00:00:00 2001 From: Ubugeeei Date: Sat, 3 Feb 2024 19:29:50 +0900 Subject: [PATCH 3/4] chore: remove dead code --- packages/runtime-vapor/src/componentProps.ts | 4 ---- 1 file changed, 4 deletions(-) diff --git a/packages/runtime-vapor/src/componentProps.ts b/packages/runtime-vapor/src/componentProps.ts index 634f38796..d7d370ceb 100644 --- a/packages/runtime-vapor/src/componentProps.ts +++ b/packages/runtime-vapor/src/componentProps.ts @@ -5,16 +5,12 @@ import { EMPTY_ARR, EMPTY_OBJ, camelize, - capitalize, extend, hasOwn, hyphenate, isArray, isFunction, - isObject, isReservedProp, - makeMap, - toRawType, } from '@vue/shared' import { shallowReactive, shallowReadonly, toRaw } from '@vue/reactivity' import type { Component, ComponentInternalInstance } from './component' From 2d2a48f9ac8a3b5678436abe9405241ea20e6079 Mon Sep 17 00:00:00 2001 From: Ubugeeei Date: Sun, 4 Feb 2024 23:04:41 +0900 Subject: [PATCH 4/4] chore: merge tests --- .../__tests__/componentProps.spec.ts | 133 +++++++++++++++-- .../__tests__/componentProps_.spec.ts | 139 ------------------ 2 files changed, 123 insertions(+), 149 deletions(-) delete mode 100644 packages/runtime-vapor/__tests__/componentProps_.spec.ts diff --git a/packages/runtime-vapor/__tests__/componentProps.spec.ts b/packages/runtime-vapor/__tests__/componentProps.spec.ts index a6860db00..52a02d1e2 100644 --- a/packages/runtime-vapor/__tests__/componentProps.spec.ts +++ b/packages/runtime-vapor/__tests__/componentProps.spec.ts @@ -232,7 +232,7 @@ describe('component props (vapor)', () => { expect(props.bar).toEqual({ a: 1 }) expect(props.baz).toEqual(defaultBaz) // expect(defaultFn).toHaveBeenCalledTimes(1) // failed: (caching is not supported) - expect(defaultFn).toHaveBeenCalledTimes(2) + expect(defaultFn).toHaveBeenCalledTimes(3) expect(defaultBaz).toHaveBeenCalledTimes(0) // #999: updates should not cause default factory of unchanged prop to be @@ -358,25 +358,138 @@ describe('component props (vapor)', () => { reset() }) - test.todo('validator', () => { - // TODO: impl validator + describe('validator', () => { + test('validator should be called with two arguments', () => { + let args: any + const mockFn = vi.fn((..._args: any[]) => { + args = _args + return true + }) + + const Comp = defineComponent({ + props: { + foo: { + type: Number, + validator: (value: any, props: any) => mockFn(value, props), + }, + bar: { + type: Number, + }, + }, + render() { + const t0 = template('

') + const n0 = t0() + return n0 + }, + }) + + const props = { + get foo() { + return 1 + }, + get bar() { + return 2 + }, + } + + render(Comp, props, host) + expect(mockFn).toHaveBeenCalled() + // NOTE: Vapor Component props defined by getter. So, `props` not Equal to `{ foo: 1, bar: 2 }` + // expect(mockFn).toHaveBeenCalledWith(1, { foo: 1, bar: 2 }) + expect(args.length).toBe(2) + expect(args[0]).toBe(1) + expect(args[1].foo).toEqual(1) + expect(args[1].bar).toEqual(2) + }) + + // TODO: impl setter and warnner + test.todo( + 'validator should not be able to mutate other props', + async () => { + const mockFn = vi.fn((...args: any[]) => true) + const Comp = defineComponent({ + props: { + foo: { + type: Number, + validator: (value: any, props: any) => !!(props.bar = 1), + }, + bar: { + type: Number, + validator: (value: any) => mockFn(value), + }, + }, + render() { + const t0 = template('
') + const n0 = t0() + return n0 + }, + }) + + render( + Comp, + { + get foo() { + return 1 + }, + get bar() { + return 2 + }, + }, + host, + ) + expect( + `Set operation on key "bar" failed: target is readonly.`, + ).toHaveBeenWarnedLast() + expect(mockFn).toHaveBeenCalledWith(2) + }, + ) }) test.todo('warn props mutation', () => { // TODO: impl warn }) - test.todo('warn absent required props', () => { - // TODO: impl warn + test('warn absent required props', () => { + const Comp = defineComponent({ + props: { + bool: { type: Boolean, required: true }, + str: { type: String, required: true }, + num: { type: Number, required: true }, + }, + setup() { + return () => null + }, + }) + render(Comp, {}, host) + expect(`Missing required prop: "bool"`).toHaveBeenWarned() + expect(`Missing required prop: "str"`).toHaveBeenWarned() + expect(`Missing required prop: "num"`).toHaveBeenWarned() }) - test.todo('warn on type mismatch', () => { - // TODO: impl warn - }) + // NOTE: type check is not supported in vapor + // test('warn on type mismatch', () => {}) // #3495 - test.todo('should not warn required props using kebab-case', async () => { - // TODO: impl warn + test('should not warn required props using kebab-case', async () => { + const Comp = defineComponent({ + props: { + fooBar: { type: String, required: true }, + }, + setup() { + return () => null + }, + }) + + render( + Comp, + { + get ['foo-bar']() { + return 'hello' + }, + }, + host, + ) + expect(`Missing required prop: "fooBar"`).not.toHaveBeenWarned() }) test('props type support BigInt', () => { diff --git a/packages/runtime-vapor/__tests__/componentProps_.spec.ts b/packages/runtime-vapor/__tests__/componentProps_.spec.ts deleted file mode 100644 index d33b93ea1..000000000 --- a/packages/runtime-vapor/__tests__/componentProps_.spec.ts +++ /dev/null @@ -1,139 +0,0 @@ -// NOTE: merge into `componentProps.spec.ts` (https://github.com/vuejs/core-vapor/pull/99) - -import { defineComponent, render, template } from '../src' - -let host: HTMLElement -const initHost = () => { - host = document.createElement('div') - host.setAttribute('id', 'host') - document.body.appendChild(host) -} -beforeEach(() => initHost()) -afterEach(() => host.remove()) - -describe('validator', () => { - test('validator should be called with two arguments', () => { - let args: any - const mockFn = vi.fn((..._args: any[]) => { - args = _args - return true - }) - - const Comp = defineComponent({ - props: { - foo: { - type: Number, - validator: (value: any, props: any) => mockFn(value, props), - }, - bar: { - type: Number, - }, - }, - render() { - const t0 = template('
') - const n0 = t0() - return n0 - }, - }) - - const props = { - get foo() { - return 1 - }, - get bar() { - return 2 - }, - } - - render(Comp, props, host) - expect(mockFn).toHaveBeenCalled() - // NOTE: Vapor Component props defined by getter. So, `props` not Equal to `{ foo: 1, bar: 2 }` - // expect(mockFn).toHaveBeenCalledWith(1, { foo: 1, bar: 2 }) - expect(args.length).toBe(2) - expect(args[0]).toBe(1) - expect(args[1].foo).toEqual(1) - expect(args[1].bar).toEqual(2) - }) - - // TODO: impl setter and warnner - test.todo('validator should not be able to mutate other props', async () => { - const mockFn = vi.fn((...args: any[]) => true) - const Comp = defineComponent({ - props: { - foo: { - type: Number, - validator: (value: any, props: any) => !!(props.bar = 1), - }, - bar: { - type: Number, - validator: (value: any) => mockFn(value), - }, - }, - render() { - const t0 = template('
') - const n0 = t0() - return n0 - }, - }) - - render( - Comp, - { - get foo() { - return 1 - }, - get bar() { - return 2 - }, - }, - host, - ) - expect( - `Set operation on key "bar" failed: target is readonly.`, - ).toHaveBeenWarnedLast() - expect(mockFn).toHaveBeenCalledWith(2) - }) - - test('warn absent required props', () => { - const Comp = defineComponent({ - props: { - bool: { type: Boolean, required: true }, - str: { type: String, required: true }, - num: { type: Number, required: true }, - }, - setup() { - return () => null - }, - }) - render(Comp, {}, host) - expect(`Missing required prop: "bool"`).toHaveBeenWarned() - expect(`Missing required prop: "str"`).toHaveBeenWarned() - expect(`Missing required prop: "num"`).toHaveBeenWarned() - }) - - // NOTE: type check is not supported in vapor - // test('warn on type mismatch', () => {}) - - // #3495 - test('should not warn required props using kebab-case', async () => { - const Comp = defineComponent({ - props: { - fooBar: { type: String, required: true }, - }, - setup() { - return () => null - }, - }) - - render( - Comp, - { - get ['foo-bar']() { - return 'hello' - }, - }, - host, - ) - expect(`Missing required prop: "fooBar"`).not.toHaveBeenWarned() - }) -})