Skip to content

POC - Render all stub slots via config #102

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

Merged
merged 6 commits into from
May 5, 2020

Conversation

dobromir-hristov
Copy link
Contributor

@dobromir-hristov dobromir-hristov commented Apr 30, 2020

This is what I got... and I am not sure I like it...

  1. Scoped slots just cannot work with auto stubs and shallowMount.
  2. We cannot know what slots a component has defined, we can just render everything.

Check it out, play with it if you want. I wanted Evan to give me an advice on this, but yeah...

Related to #69

@dobromir-hristov
Copy link
Contributor Author

I think we should make an RFC explaining why we are doing this and what solution we offer.
This is a feature that mostly worked before and breaking it will cause every shallowMount based testsuite to break. And they are not as few as we expected.

I can write up an RFC, explaining what we tried, what the limitations are and what alternatives people can use, along with what we are offering as "official" solution for the problem.

This thing should imho be in the guides later on, so people are fully aware that this way of testing is not reliable, and will lead to problems if you rely 100% on it.

@lmiller1990
Copy link
Member

Hm - we can do an RFC, sure. I feel like most of the people who have a strong opinion about rendering slots of stubbed components hold that opinion because that's how beta worked, and it will cause extra work for them to update their code-bases, not because it actually makes sense.

Could we have a flag that attempts to simulate the old behavior to let people upgrade - kind of like what you did here where a component passed to stubs will still render the default slot. Something like config.vtu-beta-compat, for example.

I understand a lot of code-bases will break, and we do need to consider those people, and this flag (might) make it easier to upgrade. I don't think re-implementing this behavior purely because that's how it worked before is the right answer.

@dobromir-hristov
Copy link
Contributor Author

Yeah, we can't really reimplement it fully anyway. It will be behind a flag, I just want to make sure people get it.

@lmiller1990
Copy link
Member

lmiller1990 commented May 2, 2020

I see two things to consider:

  • new users and the path forward. I obviously like the behavior where we just stub the component and render no children. It's simple, it works, and maintenance will be easy.
    • This seems like the easy way out (because it is) but I feel like I spend half my life triaging shallowMount and stub bugs in VTU beta, if we can avoid this I would like to do so.
  • migrating users. We can't just screw them over and change things hugely. We should "reimplement" VTU beta as best we can. Stick it behind a flag and give people 18 months to migrate.

@lmiller1990
Copy link
Member

This looks pretty good. Are we going to bundle a helper too?

Once that is done, I think we should take this chance to update the docs and explain how this works, and provide some examples on testing slots and stubs.

I can look into shallowMount soon, it should be pretty easy with this PR merged in.

Copy link
Member

@lmiller1990 lmiller1990 left a comment

Choose a reason for hiding this comment

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

This all seems good and well.

We will focus on docs for a bit now. Are you able to write about slots and stubs and shallowMount? You know it best and how it is used in large production apps.

@dobromir-hristov
Copy link
Contributor Author

Yeah ofc.

@lmiller1990
Copy link
Member

lmiller1990 commented May 5, 2020

@dobromir-hristov maybe we can add this info around here. We should also add a section about shallow: true in the mounting options. Finally, we did not include docs about global config yet.

This can be done at any time, no rush.

@lmiller1990 lmiller1990 merged commit ddb81a0 into master May 5, 2020
@lmiller1990 lmiller1990 deleted the dhristov/renderSlots-config branch May 5, 2020 07:18
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.

2 participants