fix: make scoped slots rendering consistent for stubs #2068
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
✅ Deploy Preview for vue-test-utils-v1 ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
xanf
commented
Apr 30, 2023
| getScopedSlotRenderFunctions(this) | ||
| .map(x => { | ||
| let result = null | ||
| try { |
There was a problem hiding this comment.
This try catch is best-effort attempt to mitigate migration from existing state. Since we're now rendering more scoped slots on stubs than previously there is higher chance of blow up.
While it might create some confusion (hey, some my slots are rendering, some are not) considering close EOL of Vue.js 2 I'm thinking of this one as acceptable trade-off
For example at GitLab codebase, without try - catch this MR entirely blows over 700 tests, with try catch - less than 100, however I'm open for discussion on removing it
|
Sorry, just saw this one @xanf. I'll review it now. |
lmiller1990
approved these changes
Jun 5, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This MR fixes extremely weird stubbing behavior. Currently scoped slot stubs will be rendered only for components, which are located at root level of other component. Basically:
will have scoped slots rendered on stub, while
will have only default slot rendered.
This is caused because slot lookup was performed on parent instead of instance itself
What kind of change does this PR introduce? (check at least one)
Does this PR introduce a breaking change? (check one)
This is definitely a breaking change: now we try to render more scoped slots than before. Basically, that means that some tests which rely that slots were not previously rendered will fail. However, I see this definitely a better thing than current confusing approach
The PR fulfills these requirements:
devbranch.fix #xxx[,#xxx], where "xxx" is the issue number)If adding a new feature, the PR's description includes:
Other information: