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

fix(find): allow finding root without name #836

Merged
merged 1 commit into from
Aug 29, 2021

Conversation

xanf
Copy link
Collaborator

@xanf xanf commented Aug 6, 2021

Pretty self-describing, I hope :)

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.

One comment. Also just to confirm my understanding here, the way this works is we create a stub (which is not really a stub, just maps to itself) to be able to search via constructor?

@@ -48,7 +48,7 @@ describe('getComponent', () => {
})

it('should throw if not found with a component selector that has no name', () => {
const wrapper = mount(compA)
Copy link
Member

Choose a reason for hiding this comment

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

This test is not relevant anymore, is it? It was testing for the missing behavior which you implemented.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It is still relevant - we still should throw getComponent if we were unable to find component without name. It just updated to reflect changes in this PR - previously we were searching for compA inside compA - and it throws (not the case anymore), now we search for compA inside compB and it also throws (which is what this test about - about throwing, not about matching root)

@xanf
Copy link
Collaborator Author

xanf commented Aug 10, 2021

One comment. Also just to confirm my understanding here, the way this works is we create a stub (which is not really a stub, just maps to itself) to be able to search via constructor?

Sometimes (when the component is created via options API or is a functional component) - instead of mounting the component we're providing we're making its "clone" (this is required to modify mixins field for data() and components for #845). Luckily we already have logic in place "when you're searching for component X - match componentY" which is used to allow finding stubs by original component object. This PR simply reuses this logic for root component (I'm happy that we did not introduce MORE workarounds here) :)

@lmiller1990 lmiller1990 self-requested a review August 12, 2021 13:23
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.

Great - just need to resolve the conflict.

One all these are merged up, let's do a release.

@xanf
Copy link
Collaborator Author

xanf commented Aug 12, 2021

@lmiller1990 Sure, I will arrange this today

@xanf xanf force-pushed the xanf-find-root-without-name branch from 571b00d to efc9ed7 Compare August 26, 2021 18:44
@xanf
Copy link
Collaborator Author

xanf commented Aug 26, 2021

#889 should fix current red master making this green after rebase :)
Will do as soon as #889 will be merged

@xanf xanf force-pushed the xanf-find-root-without-name branch from efc9ed7 to 3849591 Compare August 27, 2021 16:33
@xanf
Copy link
Collaborator Author

xanf commented Aug 27, 2021

@lmiller1990 🚀

@lmiller1990 lmiller1990 merged commit 6541c3d into vuejs:master Aug 29, 2021
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.

2 participants