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: stubbing RouterLink #438

Merged
merged 6 commits into from
Mar 8, 2021
Merged

fix: stubbing RouterLink #438

merged 6 commits into from
Mar 8, 2021

Conversation

lmiller1990
Copy link
Member

resolves #425

Not sure if this ideal, pretty messy but this is one way to fix the problem. 🤔 hyphenating stubs by default messes things up.

@@ -1,5 +1,6 @@
import { defineComponent } from 'vue'
import { mount } from '../src'
import { mount, MountingOptions, RouterLinkStub, shallowMount } from '../src'
import Issue425 from './components/Issue425.vue'
Copy link
Member

Choose a reason for hiding this comment

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

I love this idea

src/mount.ts Outdated
!['transition', 'transition-group'].includes(name)
) {
app.component(name, stub)
}
Copy link
Member

Choose a reason for hiding this comment

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

Can't we simply register the stub with the provided key in stubs? That's what we do for components (and I realize that's why I used this in my apps instead of stubs) and it should work fine?

Something like (untested):

 for (const [name, stub] of Object.entries(global.stubs)) {
  if (stub === true) {
    // default stub.
    app.component(name, createStub({ name, props: {} }))
  } else {
    // user has provided a custom implementation.
    app.component(name, stub)
  }

Copy link
Member Author

Choose a reason for hiding this comment

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

That is what I thought, but it was not working - seems there is something else at work here 🤔

Copy link
Member Author

Choose a reason for hiding this comment

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

See comment below - I think we can just pascal case everything and it will "just work"

src/mount.ts Outdated
// ref: https://github.com/vuejs/vue-test-utils-next/issues/249
// we register both the hyphenated version and non-hyphenated
// version for good measure.
// ref: https://github.com/vuejs/vue-test-utils-next/issues/425
if (global?.stubs) {
for (const [name, stub] of Object.entries(global.stubs)) {
const tag = hyphenate(name)
Copy link
Contributor

Choose a reason for hiding this comment

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

IIRC, when a component is registered in PascalCase, it works in the template regardless of whether the template uses kebab-case or PascalCase. maybe we could solve this without double registration by replacing this with a toPascalCase()?

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm neat idea, I will give this a try

Co-authored-by: Alex Van Liew <[email protected]>
@snoozbuster
Copy link
Contributor

Question: how does config.renderStubDefaultSlot work? I was having trouble getting that to work in my project as well (where we are using 100% PascalCase) and now that I have the additional context of this PR, I am wondering if that also wasn't working because the stubs with the default slot rendering were registered as kebab-case...

@lmiller1990
Copy link
Member Author

renderDefaultSlot is a flag for back-compat with a somewhat unintended feature in v1. Basically in this lib when you stub a component, it's entirely is stubbed - including and slots it might otherwise render.

That flag can force the default slot to be rendered. There is an example documented here (note the shallow: true mounting option, which is the same as shallowMount.

It looks like if you use components instead of stubs your test works as-is - this bug only seems to apply to stubs (which will be fixed in this PR).

@lmiller1990 lmiller1990 requested a review from cexbrayat March 6, 2021 10:54
@lmiller1990
Copy link
Member Author

Fixed, I just removed hyphenate and it works like suggested. Pls re-review @snoozbuster @afontcu @cexbrayat.

Copy link
Member

@cexbrayat cexbrayat left a comment

Choose a reason for hiding this comment

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

That's what I had in mind 👍
There is a comment that needs to be fixed.

And it would be great to squash all this into a single commit before merging 😉

Co-authored-by: Alex Van Liew <[email protected]>
@lmiller1990
Copy link
Member Author

@cexbrayat added the comment thanks to @snoozbuster.

I will use the squash/merge feature on GH to squash all the commits.

@lmiller1990 lmiller1990 merged commit bb8023f into master Mar 8, 2021
@lmiller1990 lmiller1990 deleted the fix-stubbing-router-link branch March 8, 2021 12:03
@lmiller1990
Copy link
Member Author

It's merged. I will probably do a release this weekend, we should get a few more things fixed by then. Thanks all!

@snoozbuster
Copy link
Contributor

@lmiller1990 could we release sooner than this weekend? some of the tests I am trying to write are getting stuck on this issue, and I don't want to put in a workaround that I know I will have to come back and fix very soon.

@lmiller1990
Copy link
Member Author

Alright, please wait for 24 hours or so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cannot stub RouterLink using RouterLinkStub in tests when PascalCase is used for RouterLink in component
4 participants