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
fix: remove symbol props from stubs #1086
Changes from 7 commits
d11f04b
84670a6
4ddcd22
e0f372f
f051b60
63514b4
deee71e
232bf57
a54b1da
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,7 +7,8 @@ import { | |
defineComponent, | ||
VNodeTypes, | ||
ConcreteComponent, | ||
ComponentPropsOptions | ||
ComponentPropsOptions, | ||
ComponentObjectPropsOptions | ||
} from 'vue' | ||
import { hyphenate } from './utils/vueShared' | ||
import { matchName } from './utils/matchName' | ||
|
@@ -21,7 +22,7 @@ import { Stub, Stubs } from './types' | |
|
||
interface StubOptions { | ||
name: string | ||
propsDeclaration?: ComponentPropsOptions | ||
propsDeclaration: ComponentPropsOptions | ||
renderStubDefaultSlot?: boolean | ||
} | ||
|
||
|
@@ -42,6 +43,17 @@ const shouldNotStub = (type: ConcreteComponent) => doNotStubComponents.has(type) | |
export const addToDoNotStubComponents = (type: ConcreteComponent) => | ||
doNotStubComponents.add(type) | ||
|
||
const removeSymbols = <T = ComponentPropsOptions>(props: T): T => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: this could be renamed to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ooc: is the generic type There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Removed that |
||
// props are always normalized to object syntax | ||
const $props = props as unknown as ComponentObjectPropsOptions | ||
return Object.keys($props).reduce((acc, key) => { | ||
if (typeof $props[key] === 'symbol') { | ||
return { ...acc, [key]: $props[key]?.toString() } | ||
} | ||
return { ...acc, [key]: $props[key] } | ||
}, {}) as T | ||
} | ||
|
||
export const createStub = ({ | ||
name, | ||
propsDeclaration, | ||
|
@@ -51,7 +63,19 @@ export const createStub = ({ | |
const tag = name ? `${hyphenate(name)}-stub` : anonName | ||
|
||
const render = (ctx: ComponentPublicInstance) => { | ||
return h(tag, ctx.$props, renderStubDefaultSlot ? ctx.$slots : undefined) | ||
// https://github.com/vuejs/vue-test-utils-next/issues/1076 | ||
// Passing a symbol as a static prop is not legal, since Vue will try to do | ||
// something like `el.setAttribute('val', Symbol())` which is not valid and | ||
// causes an error. | ||
// Only a problem when shallow mounting. For this reason we iterate of the | ||
// props that will be passed and remove any that are symbols. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. description is not accurate anymore: this should mention that they are stringified |
||
const propsWithoutSymbols = removeSymbols(ctx.$props) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: same here, the variable could be renamed |
||
|
||
return h( | ||
tag, | ||
propsWithoutSymbols, | ||
renderStubDefaultSlot ? ctx.$slots : undefined | ||
) | ||
} | ||
|
||
return defineComponent({ | ||
|
@@ -164,7 +188,8 @@ export function stubComponents( | |
if (type === Transition && 'transition' in stubs && stubs['transition']) { | ||
return [ | ||
createTransitionStub({ | ||
name: 'transition-stub' | ||
name: 'transition-stub', | ||
propsDeclaration: {} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If the option is reverted to optional, this can be removed. |
||
}), | ||
undefined, | ||
children | ||
|
@@ -179,7 +204,8 @@ export function stubComponents( | |
) { | ||
return [ | ||
createTransitionStub({ | ||
name: 'transition-group-stub' | ||
name: 'transition-group-stub', | ||
propsDeclaration: {} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same here |
||
}), | ||
undefined, | ||
children | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
<template> | ||
<div>Symbol: {{ $props.sym }}</div> | ||
</template> | ||
|
||
<script lang="ts" setup> | ||
interface Props { | ||
sym?: Symbol | ||
} | ||
|
||
withDefaults(defineProps<Props>(), { | ||
sym: () => Symbol() | ||
}) | ||
</script> |
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.
is this necessary? Can't that be left optional, and have a default value in the stubs?
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.
Done, fixed all comments, thanks for the review!