Skip to content

fix #1377 string stubs dropping props #1473

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

Merged
merged 5 commits into from
Mar 26, 2020
Merged

fix #1377 string stubs dropping props #1473

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
aebcc13
test(issue #1377): add test for props on stubbed child component
Mar 9, 2020
68640c1
feat(create-instance): convert string stubs to template
Mar 17, 2020
34aadf2
docs(api): update stubs documentation
Mar 17, 2020
30428b4
test(stubs): limit stub props to 2.2 and above
Mar 19, 2020
2bd67b6
docs(api): updated docs wording
Mar 25, 2020
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
2 changes: 1 addition & 1 deletion docs/api/mount.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ describe('Foo', () => {
it('renders a div', () => {
const wrapper = mount(Foo, {
stubs: {
Bar: '<div class="stubbed" />',
Bar: '<div class="stubbed" />', // DEPRECATED
Copy link
Member

Choose a reason for hiding this comment

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

if it is deprecated should we just nuke it from the docs?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'd make an argument against that as the typescript definitions show strings as a valid option. It's very frustrating when you see something in the code, but missing in the documentation. Is there a better place to indicate that strings are deprecated?

Copy link
Member

@lmiller1990 lmiller1990 Mar 18, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Right, makes sense. Not sure.... strings aren't really deprecated, just ones that are valid HTML. Could we just add a check and throw and error if the string includes < or >, since you can still pass something like "Text". Or are we dropping that entirely too? In that case we would change the definitions and remove string?

Copy link
Contributor Author

@Stoom Stoom Mar 19, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

If you were to just pass text then the vue template compiler will complain about it not being a valid template.

  options.stub with mount
[vue-test-utils]: String stubs are deprecated and will be removed in future versions
[Vue warn]: Error compiling template:

Hello

- Component template requires a root element, rather than just text.

Copy link
Member

Choose a reason for hiding this comment

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

Right, I wasn't thinking. You'd need to pass h('text').

Let's just drop it, and remove string from the TS definitions if it's not longer valid? Or am I missing something 🤔

Copy link
Member

Choose a reason for hiding this comment

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

I think making breaking changes before 1.0 make a lot more sense - once we hit 1.0, deprecating things becomes a lot more difficult.

What do you think if we just deprecate it right now and add a check + warning "This is now deprecated"?

Either way let's make a decision and get this merged up! I'll leave it up to you to make the final call.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Deprecated as in still works for maybe the next release or two. then rip it out? I'm good with that. Really it's just to give people a chance to migrate without completely breaking any of these stubs.

Completely agree with after 1.0 it's a pain to get something out. I'd suggest that we remove this completely before 1.0 final release. Any idea on how many more beta's there will be?

Copy link
Contributor

Choose a reason for hiding this comment

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

Just show the right methods here, and add a note below, of what used to work, but is now deprecated.

New users get to read the valid syntax, and those that are searching will find why its erroring out when they update :)

Copy link
Member

Choose a reason for hiding this comment

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

Sounds good. Can we get this added and merge this @Stoom , thanks for sticking with this PR for so long

I have no idea about the number of betas, since I think are unclear on what constitutes a 1.0, I think we should get Vue 3 support working (it's WIP, hopefully public alpha soon) and call whatever we have at that point the 1.0 release.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Uh are we dropping string templates entirely...?

BarFoo: true,
FooBar: Faz
}
Expand Down
5 changes: 4 additions & 1 deletion docs/api/options.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -208,10 +208,13 @@ const wrapper = mount(WrapperComp).find(ComponentUnderTest)

## stubs

- type: `{ [name: string]: Component | boolean } | Array<string>`
- type: `{ [name: string]: Component | string | boolean } | Array<string>`

Stubs child components. Can be an Array of component names to stub, or an object. If `stubs` is an Array, every stub is `<${component name}-stub>`.

Depercation note:
If declaring an object, string stubs are now deprecated and will be removed in a future version.

Copy link
Member

Choose a reason for hiding this comment

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

I think we can just drop it entirely

Copy link
Contributor

Choose a reason for hiding this comment

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

Because we dont have versioned docs, I think it makes sense to leave it. We are deprecating, but have not yet removed it. (We saw what removing sync did without mentioning it in the docs any more).

Maybe we can add an example what is now deprecated?

Example:

```js
Expand Down
43 changes: 26 additions & 17 deletions packages/create-instance/create-component-stubs.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@ import {
camelize,
capitalize,
hyphenate,
keys
keys,
warn
} from '../shared/util'
import {
componentNeedsCompiling,
Expand All @@ -15,7 +16,7 @@ import {
isDynamicComponent,
isConstructor
} from '../shared/validators'
import { compileTemplate, compileFromString } from '../shared/compile-template'
import { compileTemplate } from '../shared/compile-template'

function isVueComponentStub(comp): boolean {
return (comp && comp.template) || isVueComponent(comp)
Expand Down Expand Up @@ -145,24 +146,31 @@ export function createStubFromComponent(
}
}

function createStubFromString(
templateString: string,
originalComponent: Component = {},
name: string,
_Vue: Component
): Component {
// DEPRECATED: converts string stub to template stub.
function createStubFromString(templateString: string, name: string): Component {
warn('String stubs are deprecated and will be removed in future versions')

if (templateContainsComponent(templateString, name)) {
throwError('options.stub cannot contain a circular reference')
}
const componentOptions = resolveOptions(originalComponent, _Vue)

return {
...getCoreProperties(componentOptions),
$_doNotStubChildren: true,
...compileFromString(templateString)
template: templateString,
$_doNotStubChildren: true
}
}

function setStubComponentName(
stub: Object,
originalComponent: Component = {},
_Vue: Component
) {
if (stub.name) return

const componentOptions = resolveOptions(originalComponent, _Vue)
stub.name = getCoreProperties(componentOptions).name
}

function validateStub(stub) {
if (!isValidStub(stub)) {
throwError(`options.stub values must be passed a string or ` + `component`)
Expand All @@ -175,26 +183,27 @@ export function createStubsFromStubsObject(
_Vue: Component
): Components {
return Object.keys(stubs || {}).reduce((acc, stubName) => {
const stub = stubs[stubName]
let stub = stubs[stubName]

validateStub(stub)

if (stub === false) {
return acc
}

const component = resolveComponent(originalComponents, stubName)

if (stub === true) {
const component = resolveComponent(originalComponents, stubName)
acc[stubName] = createStubFromComponent(component, stubName, _Vue)
return acc
}

if (typeof stub === 'string') {
const component = resolveComponent(originalComponents, stubName)
acc[stubName] = createStubFromString(stub, component, stubName, _Vue)
return acc
stub = createStubFromString(stub, stubName)
stubs[stubName]
}

setStubComponentName(stub, component, _Vue)
if (componentNeedsCompiling(stub)) {
compileTemplate(stub)
}
Expand Down
24 changes: 12 additions & 12 deletions packages/shared/compile-template.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,19 +4,16 @@ import { compileToFunctions } from 'vue-template-compiler'
import { componentNeedsCompiling } from './validators'
import { throwError } from './util'

export function compileFromString(str: string) {
if (!compileToFunctions) {
throwError(
`vueTemplateCompiler is undefined, you must pass ` +
`precompiled components if vue-template-compiler is ` +
`undefined`
)
}
return compileToFunctions(str)
}

export function compileTemplate(component: Component): void {
if (component.template) {
if (!compileToFunctions) {
throwError(
`vueTemplateCompiler is undefined, you must pass ` +
`precompiled components if vue-template-compiler is ` +
`undefined`
)
}

if (component.template.charAt('#') === '#') {
var el = document.querySelector(component.template)
if (!el) {
Expand All @@ -27,7 +24,10 @@ export function compileTemplate(component: Component): void {
component.template = el.innerHTML
}

Object.assign(component, compileToFunctions(component.template))
Object.assign(component, {
...compileToFunctions(component.template),
name: component.name
})
}

if (component.components) {
Expand Down
22 changes: 22 additions & 0 deletions test/resources/components/component-with-nested-childern-and-attributes.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
<template>
<div>
<span>
<slot-component prop1="foobar" prop2="fizzbuzz" />
<child-component prop1="foobar" prop2="fizzbuzz" />
<original-component prop1="foobar" prop2="fizzbuzz" />
</span>
</div>
</template>

<script>
import ComponentWithProps from './component-with-props.vue'

export default {
name: 'component-with-nested-children',
components: {
ChildComponent: ComponentWithProps,
SlotComponent: ComponentWithProps,
OriginalComponent: ComponentWithProps
}
}
</script>
63 changes: 58 additions & 5 deletions test/specs/mounting-options/stubs.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import ComponentWithChild from '~resources/components/component-with-child.vue'
import ComponentWithNestedChildren from '~resources/components/component-with-nested-children.vue'
import Component from '~resources/components/component.vue'
import ComponentAsAClass from '~resources/components/component-as-a-class.vue'
import ComponentWithNestedChildrenAndAttributes from '~resources/components/component-with-nested-childern-and-attributes.vue'
import { createLocalVue, config } from '@vue/test-utils'
import { config as serverConfig } from '@vue/server-test-utils'
import Vue from 'vue'
Expand All @@ -18,6 +19,7 @@ describeWithShallowAndMount('options.stub', mountingMethod => {
serverConfigSave = serverConfig.stubs
config.stubs = {}
serverConfig.stubs = {}
sandbox.stub(console, 'error').callThrough()
})

afterEach(() => {
Expand All @@ -32,21 +34,24 @@ describeWithShallowAndMount('options.stub', mountingMethod => {
const ComponentWithoutRender = { template: '<div></div>' }
const ExtendedComponent = { extends: ComponentWithRender }
const SubclassedComponent = Vue.extend({ template: '<div></div>' })
const StringComponent = '<div></div>'
mountingMethod(ComponentWithChild, {
stubs: {
ChildComponent: ComponentWithRender,
ChildComponent2: ComponentAsAClass,
ChildComponent3: ComponentWithoutRender,
ChildComponent4: ExtendedComponent,
ChildComponent5: SubclassedComponent
ChildComponent5: SubclassedComponent,
ChildComponent6: StringComponent
}
})
})

it('replaces component with template string ', () => {
const Stub = { template: '<div class="stub"></div>' }
const wrapper = mountingMethod(ComponentWithChild, {
stubs: {
ChildComponent: '<div class="stub"></div>'
ChildComponent: Stub
}
})
expect(wrapper.findAll('.stub').length).to.equal(1)
Expand Down Expand Up @@ -321,7 +326,7 @@ describeWithShallowAndMount('options.stub', mountingMethod => {

const wrapper = mountingMethod(TestComponent, {
stubs: {
'span-component': '<p />'
'span-component': { template: '<p />' }
},
localVue
})
Expand All @@ -342,7 +347,7 @@ describeWithShallowAndMount('options.stub', mountingMethod => {

const wrapper = mountingMethod(TestComponent, {
stubs: {
'time-component': '<span />'
'time-component': { template: '<span />' }
},
localVue
})
Expand Down Expand Up @@ -414,7 +419,7 @@ describeWithShallowAndMount('options.stub', mountingMethod => {
expect(wrapper.html()).contains('No render function')
})

it('throws an error when passed a circular reference', () => {
it('throws an error when passed a circular reference for string stubs', () => {
const names = ['child-component', 'ChildComponent', 'childComponent']
const validValues = [
'<NAME-suffix />',
Expand Down Expand Up @@ -590,4 +595,52 @@ describeWithShallowAndMount('options.stub', mountingMethod => {
expect(result.props().propA).to.equal('A')
delete Vue.options.components['child-component']
})

itRunIf(
vueVersion >= 2.2,
'renders props in the element as attributes',
() => {
const ComponentStub = { template: '<div id="component-stub" />' }
const StringStub = '<div id="string-stub" />'
const BooleanStub = true

const wrapper = mountingMethod(ComponentWithNestedChildrenAndAttributes, {
stubs: {
SlotComponent: ComponentStub,
ChildComponent: StringStub,
OriginalComponent: BooleanStub
}
})

expect(wrapper.find('#component-stub').attributes()).to.eql({
id: 'component-stub',
prop1: 'foobar',
prop2: 'fizzbuzz'
})
expect(wrapper.find('#string-stub').attributes()).to.eql({
id: 'string-stub',
prop1: 'foobar',
prop2: 'fizzbuzz'
})
expect(wrapper.find('originalcomponent-stub').attributes()).to.eql({
prop1: 'foobar',
prop2: 'fizzbuzz'
})
}
)

it('warns when passing a string', () => {
const StringComponent = '<div></div>'
mountingMethod(ComponentWithChild, {
stubs: {
ChildComponent6: StringComponent
}
})

expect(console.error).calledWith(
sandbox.match(
'[vue-test-utils]: String stubs are deprecated and will be removed in future versions'
)
)
})
})