fix #1377 string stubs dropping props #1473

Stoom · 2020-03-17T05:53:39Z

What kind of change does this PR introduce? (check at least one)

Does this PR introduce a breaking change? (check one)

Yes
No

If yes, please describe the impact and migration path for existing applications:

The PR fulfills these requirements:

It's submitted to the dev branch.
When resolving a specific issue, it's referenced in the PR's title (e.g. fix #xxx[,#xxx], where "xxx" is the issue number)
All tests are passing: https://github.com/vuejs/vue-test-utils/blob/dev/.github/CONTRIBUTING.md#development-setup
New/updated tests are included

Other information:
This deprecates using a string stub. It fixes a missing check on the compile function along with the dropping of the component name on template stubs.

close vuejs#1377 This also fixes the component name being dropped for template stubs

Stoom · 2020-03-17T17:28:08Z

Any thoughts on why it would be failing with Karma?

lmiller1990

Not sure how to handle deprecations, we really need a process for this.

Also not sure why karma is failing, can take a look this week. It is failing in the compat step, which runs the tests old older version of Vue. You should be able to run these locally too, there is a script (with compat) in the name. Alternatively you can just install older versions of Vue and see which one failing, starting from 2.0. There is a helper somewhere to skips tests that don't make sense for older versions (eg mainly slots had some breaking changes along the way).

Great job on this!

lmiller1990 · 2020-03-17T23:20:57Z

docs/api/mount.md

@@ -116,7 +116,7 @@ describe('Foo', () => {
  it('renders a div', () => {
    const wrapper = mount(Foo, {
      stubs: {
-        Bar: '<div class="stubbed" />',
+        Bar: '<div class="stubbed" />', // DEPRECATED


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

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?

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?

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.

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 🤔

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.

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?

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 :)

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.

Uh are we dropping string templates entirely...?

lmiller1990 · 2020-03-17T23:21:32Z

docs/api/options.md


 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.
+


I think we can just drop it entirely

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?

dobromir-hristov · 2020-03-23T15:00:43Z

docs/api/options.md


 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.
+


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?

dobromir-hristov · 2020-03-23T15:01:37Z

docs/api/mount.md

@@ -116,7 +116,7 @@ describe('Foo', () => {
  it('renders a div', () => {
    const wrapper = mount(Foo, {
      stubs: {
-        Bar: '<div class="stubbed" />',
+        Bar: '<div class="stubbed" />', // DEPRECATED


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 :)

Stoom · 2020-03-25T13:47:58Z

@lmiller1990 @dobromir-hristov I've updated to docs. Let me know what you think, or how you would change it 🙂

dobromir-hristov · 2020-03-25T13:54:49Z

lgtm

Jamie Stumme added 3 commits March 9, 2020 11:32

test(issue vuejs#1377): add test for props on stubbed child component

aebcc13

feat(create-instance): convert string stubs to template

68640c1

close vuejs#1377 This also fixes the component name being dropped for template stubs

docs(api): update stubs documentation

34aadf2

lmiller1990 reviewed Mar 17, 2020

View reviewed changes

StummeJ mentioned this pull request Mar 18, 2020

Shallow mount drops attributes on stubbed components #1377

Closed

test(stubs): limit stub props to 2.2 and above

30428b4

Stoom force-pushed the dev branch from aaa0f53 to 30428b4 Compare March 19, 2020 12:51

lmiller1990 mentioned this pull request Mar 23, 2020

Add v-slot support in scopedSlots property, fix #1457 #1485

Merged

12 tasks

dobromir-hristov approved these changes Mar 23, 2020

View reviewed changes

docs(api): updated docs wording

2bd67b6

lmiller1990 merged commit 0c4a811 into vuejs:dev Mar 26, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix #1377 string stubs dropping props #1473

fix #1377 string stubs dropping props #1473

Stoom commented Mar 17, 2020 •

edited

Loading

Stoom commented Mar 17, 2020

lmiller1990 left a comment •

edited

Loading

lmiller1990 Mar 17, 2020

Stoom Mar 18, 2020

lmiller1990 Mar 18, 2020 •

edited

Loading

Stoom Mar 19, 2020 •

edited

Loading

lmiller1990 Mar 20, 2020

lmiller1990 Mar 23, 2020

Stoom Mar 23, 2020

dobromir-hristov Mar 23, 2020

lmiller1990 Mar 25, 2020

JessicaSachs Apr 28, 2020

lmiller1990 Mar 17, 2020

dobromir-hristov Mar 23, 2020

dobromir-hristov Mar 23, 2020

dobromir-hristov Mar 23, 2020

Stoom commented Mar 25, 2020

dobromir-hristov commented Mar 25, 2020

fix #1377 string stubs dropping props #1473

fix #1377 string stubs dropping props #1473

Conversation

Stoom commented Mar 17, 2020 • edited Loading

Stoom commented Mar 17, 2020

lmiller1990 left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lmiller1990 Mar 18, 2020 • edited Loading

Choose a reason for hiding this comment

Stoom Mar 19, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Stoom commented Mar 25, 2020

dobromir-hristov commented Mar 25, 2020

Stoom commented Mar 17, 2020 •

edited

Loading

lmiller1990 left a comment •

edited

Loading

lmiller1990 Mar 18, 2020 •

edited

Loading

Stoom Mar 19, 2020 •

edited

Loading