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

Found and fix a mount issue with find #230

Conversation

CurtisHumphrey
Copy link
Contributor

Found and fix a mount issue with find when using a named component e.g. find('Foo')

Added test case to highlight the issue

fixed someone else's lint issues too

@ljharb
Copy link
Member

ljharb commented Mar 3, 2016

Thanks - could you squash these two commits?

@CurtisHumphrey CurtisHumphrey force-pushed the mount_find_component_by_name branch from 121ccdf to d8c021f Compare March 4, 2016 14:44
@CurtisHumphrey
Copy link
Contributor Author

@ljharb Ok squashed

@lelandrichardson
Copy link
Collaborator

@CurtisHumphrey thanks for finding this bug! I'm not sure we should be using the ShallowTraversal helpers with mount, though. Can you pull in MountedTraversal instead and use the instHasType helper function and see if that works?

@CurtisHumphrey
Copy link
Contributor Author

@lelandrichardson The instHasType helper in MountedTraversal is where the problem was. The same logic from ShallowTraversal's nodeHasType solved the problem and I was trying to keep it dry. I did not find anything in MountedTraversal that did the same check

@lelandrichardson
Copy link
Collaborator

@CurtisHumphrey we should be adding code to the instHasType helper, then. We should try not to mix the logic between MountedTraversal and ShallowTraversal.

@CurtisHumphrey
Copy link
Contributor Author

@lelandrichardson So you would prefer copy any paste? If so I can do that. Or I guess we could move the function into the utils file

@CurtisHumphrey CurtisHumphrey force-pushed the mount_find_component_by_name branch from d8c021f to ed9398c Compare March 7, 2016 14:55
@CurtisHumphrey
Copy link
Contributor Author

@lelandrichardson ok, I move the function into util, rebased and squashed

@aweary
Copy link
Collaborator

aweary commented Mar 7, 2016

@CurtisHumphrey it looks like you might have added some Sublime project files to your commit.

@CurtisHumphrey CurtisHumphrey force-pushed the mount_find_component_by_name branch from ed9398c to 2d69c0d Compare March 7, 2016 15:47
@CurtisHumphrey
Copy link
Contributor Author

@aweary, remove them oops.

@lelandrichardson
Copy link
Collaborator

Looks legit. Thanks!

lelandrichardson added a commit that referenced this pull request Mar 7, 2016
@lelandrichardson lelandrichardson merged commit 907fe34 into enzymejs:master Mar 7, 2016
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.

4 participants