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

Refactor: refactor stub-components.js #544

Merged
merged 1 commit into from Apr 24, 2018
Merged

Refactor: refactor stub-components.js #544

merged 1 commit into from Apr 24, 2018

Conversation

lusarz
Copy link
Contributor

@lusarz lusarz commented Apr 20, 2018

Pull request contains changes like

  • separate stub options validation from creation of stubs
  • overall cleaning

if (component.extendOptions && component.extendOptions.components) {
stubComponents(component.extendOptions.components, stubbedComponents)
}
export function createComponentStubs (components: Object = {}, stubOptions: Object): Object {
Copy link
Contributor Author

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

Copy link
Member

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

Copy link
Contributor Author

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

delete components[c]._Ctor // eslint-disable-line no-param-reassign
})
return components
if (Vue.config.ignoredElements) {
Copy link
Contributor Author

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.

Copy link
Member

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 :)

Copy link
Contributor Author

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)) {
Copy link
Member

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>'

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed :)

@eddyerburgh
Copy link
Member

Thanks :)

@eddyerburgh eddyerburgh merged commit db5e07e into vuejs:dev Apr 24, 2018
eddyerburgh pushed a commit that referenced this pull request May 5, 2018
* 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
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

2 participants