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.

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

Merged
merged 9 commits into from Dec 13, 2021
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
36 changes: 31 additions & 5 deletions src/stubs.ts
Expand Up @@ -7,7 +7,8 @@ import {
defineComponent,
VNodeTypes,
ConcreteComponent,
ComponentPropsOptions
ComponentPropsOptions,
ComponentObjectPropsOptions
} from 'vue'
import { hyphenate } from './utils/vueShared'
import { matchName } from './utils/matchName'
Expand All @@ -21,7 +22,7 @@ import { Stub, Stubs } from './types'

interface StubOptions {
name: string
propsDeclaration?: ComponentPropsOptions
propsDeclaration: ComponentPropsOptions
Copy link
Member

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?

Copy link
Member Author

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!

renderStubDefaultSlot?: boolean
}

Expand All @@ -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 => {
Copy link
Member

Choose a reason for hiding this comment

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

nit: this could be renamed to stringifySymbols as it is not removing them anymore?

Copy link
Member

Choose a reason for hiding this comment

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

ooc: is the generic type T necessary here? As it is only called once, can't we use ComponentPropsOptions directly?

Copy link
Member Author

Choose a reason for hiding this comment

The 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,
Expand All @@ -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.
Copy link
Member

Choose a reason for hiding this comment

The 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)
Copy link
Member

Choose a reason for hiding this comment

The 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({
Expand Down Expand Up @@ -164,7 +188,8 @@ export function stubComponents(
if (type === Transition && 'transition' in stubs && stubs['transition']) {
return [
createTransitionStub({
name: 'transition-stub'
name: 'transition-stub',
propsDeclaration: {}
Copy link
Member

Choose a reason for hiding this comment

The 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
Expand All @@ -179,7 +204,8 @@ export function stubComponents(
) {
return [
createTransitionStub({
name: 'transition-group-stub'
name: 'transition-group-stub',
propsDeclaration: {}
Copy link
Member

Choose a reason for hiding this comment

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

same here

}),
undefined,
children
Expand Down
13 changes: 13 additions & 0 deletions tests/components/PropWithSymbol.vue
@@ -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>
54 changes: 53 additions & 1 deletion tests/props.spec.ts
@@ -1,5 +1,6 @@
import { mount } from '../src'
import { mount, shallowMount } from '../src'
import WithProps from './components/WithProps.vue'
import PropWithSymbol from './components/PropWithSymbol.vue'
import Hello from './components/Hello.vue'
import { defineComponent, h } from 'vue'

Expand Down Expand Up @@ -224,4 +225,55 @@ describe('props', () => {

expect(wrapper.text()).toEqual('hello')
})

describe('edge case with symbol props and stubs', () => {
it('works with Symbol as default', () => {
const Comp = defineComponent({
template: `<div>Symbol: {{ sym }}</div>`,
props: {
sym: {
type: Symbol,
default: () => Symbol()
}
}
})

const wrapper = shallowMount(Comp)

expect(wrapper.html()).toBe('<div>Symbol: Symbol()</div>')
})

it('works with symbol as array syntax', () => {
const Comp = defineComponent({
name: 'Comp',
template: `<div>Symbol: {{ sym }}</div>`,
props: ['sym']
})

const wrapper = shallowMount({
render() {
return h(Comp, { sym: Symbol() })
}
})

expect(wrapper.html()).toBe('<comp-stub sym="Symbol()"></comp-stub>')
})

it('works with symbol as default from SFC', () => {
const App = defineComponent({
template: `<PropWithSymbol :sym="sym" />`,
components: { PropWithSymbol },
data() {
return {
sym: Symbol()
}
}
})
const wrapper = shallowMount(App)

expect(wrapper.html()).toBe(
'<prop-with-symbol-stub sym="Symbol()"></prop-with-symbol-stub>'
)
})
})
})