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

Feat: Implement Scoped Slot string support #115

Merged
merged 11 commits into from May 12, 2020

Conversation

dobromir-hristov
Copy link
Contributor

This PR implements the previously very convenient scoped slot syntax of providing just a template string.

@lmiller1990
Copy link
Member

I played with this, seems good. I wonder if we need any tests with shallowMount and the renderDefaultSlot flag?

PS: I fixed the build. We had to specify @vue/compiler-dom as an external dependency the rollup config. This means if you want to use VTU in a browser as an IIFE, you will need to include VueCompilerDOM globally. I think this makes sense.

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 is great! Thanks @znck and @dobromir-hristov.

@dobromir-hristov
Copy link
Contributor Author

OK yeah awesome. As I said I did not have time to spend on it, but glad you figured it out :)))

I think we should make note in the docs that it needs to be present. Jessica's Vite toy project would need it.

Overall I am very very happy with how this turned out. It reduced the complexity by N times, everything is handled by the Vue compiler. Sure we have a hacky string to function thing, but thats fine 😆

I can add more tests if anyone thinks of something? Maybe some not so happy paths?

@lmiller1990
Copy link
Member

lmiller1990 commented May 10, 2020

I have updated the rollup config further here: https://github.com/vuejs/vue-test-utils-next/pulls

We have it listed as a peer dependency, I think this should be okay. For Vite, it will work out of the box, since all the packages in vue-next have the module field. only people using this as an IIFE will need to be aware of this (not sure how common this is, now that es modules work in modern browsers).

Sure, feel free to add more tests - would be happy to merge as is, as well. I'll leave this decision up to you since you own this feature 👍

@znck
Copy link
Member

znck commented May 10, 2020

We can't avoid string to function conversion, even full build of Vue uses the same thing:

https://github.com/vuejs/vue-next/blob/b725b63e5fd6b9c9875002c2c23e210293bd771b/packages/vue/src/index.ts#L66-L68

@dobromir-hristov dobromir-hristov merged commit 7ebbc68 into master May 12, 2020
@dobromir-hristov dobromir-hristov deleted the fix/scoped-slots-support branch May 12, 2020 06:58
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.

None yet

3 participants