-
Notifications
You must be signed in to change notification settings - Fork 668
Refactor: refactor stub-components.js #544
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
Conversation
if (component.extendOptions && component.extendOptions.components) { | ||
stubComponents(component.extendOptions.components, stubbedComponents) | ||
} | ||
export function createComponentStubs (components: Object = {}, stubOptions: Object): Object { |
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.
Personally I think public methods should be moved to top of file. Didn't do it to satisfy git blame
as much as possible
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 don't think we should move the position of functions. Most files in this project have public functions at the bottom of the files
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.
Ok, let it stay like that
packages/shared/stub-components.js
Outdated
delete components[c]._Ctor // eslint-disable-line no-param-reassign | ||
}) | ||
return components | ||
if (Vue.config.ignoredElements) { |
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.
Do we need it ? I copied it from original source.
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 don't think we do. you can remove it :)
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.
Removed :)
throwError('vueTemplateCompiler is undefined, you must pass components explicitly if vue-template-compiler is undefined') | ||
} | ||
|
||
if (containsTemplateWithCircularReference(stubOptions, components)) { |
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.
This is actually causing bugs for some users. The check is not strict enough, so I've removed the check in the source code for the moment. Could you either remove this code, or improve the code to handle edge cases like: UnreadState: '<div data-stub-unread-state v-bind="$props"><slot/></div>'
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.
Removed :)
Thanks :) |
* Revert "refactor: refactor stub-components.js (#544)" This reverts commit db5e07e. * Revert "refactor: refactor add-slots.js (#556)" This reverts commit ce9e1bf. * delete packages/test-utils/src/shallow.js * add functions * add type SlotValue * add test * fix slotValue * fix validateEnvironment * add test * fix lint * Update slots.spec.js
Pull request contains changes like