-
Notifications
You must be signed in to change notification settings - Fork 264
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(config): Do not use config.renderStubDefaultSlot #1797
Conversation
✅ Deploy Preview for vue-test-utils-docs ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
93a6a3d
to
d16ceb7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is fine - I don't think that many people are using this, except people who know VTU really well.
Can you write a short and simple explanation for this so we can include it in the patch notes @xanf? Then we can merge and release.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
I don't think this is too breaking, but maybe we could have an intermediate state that does not remove the option but logs a warning about its deprecation and indicates that config.global.renderStubDefaultSlot
should be preferred? I think marking it with @deprecated
can also help in the meantime, as people can spot the deprecation via their IDE or ESLint. Then we could really remove the option in the following release (maybe v2.1)
But I'm fine with going straight ahead as well if it is properly documented, as I don't think this is widely used.
* docs mention config.global.renderStubDefaultSlot * get rid of double definition
d16ceb7
to
e5b7c9c
Compare
@lmiller1990 @cexbrayat I've updated PR with your suggestions:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
Right now I've spent 30 minutes figuring why my default slots in stubs were not rendered, just to figure out, that our docs mention
global.renderStubDefaultSlot
as the way to control stub behavior, however we have bothconfig.renderStubDefaultSlot
andconfig.global.renderStubDefaultSlot
with first being a preferenceWhile technically this is a breaking change, I find an ability to define same thing in two places extremely confusing and fix for affected codebases will be trivial (swap
config.renderStubDefaultSlot
withconfig.global.renderStubDefaultSlot
)Otherwise, if we decide that we do not want to have such breaking change, I suggest swapping priority, so
config.global.renderStubDefaultSlot
will be taken into account beforeconfig.renderStubDefaultSlot
and markconfig.renderStubDefaultSlot
as deprecated