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

Add context to react-tree-walker call #49

Merged
merged 1 commit into from
Dec 14, 2018

Conversation

capellini
Copy link
Contributor

@capellini capellini commented Dec 13, 2018

By default react-tree-walker passes in an undefined context, which
breaks packages like Material UI and JSSProvider. This fix passes
in an empty object for context, which fixes components that use the
legacy context API.

See

By default react-tree-walker passes in an undefined context, which
breaks packages like @material-ui and JSSProvider.  This fix passes
in an empty object for context, which fixes components that use the
legacy context API.
@capellini
Copy link
Contributor Author

Note that I'm still having trouble using the new React Context API, but it seems others are having the same problem, see ctrlplusb/react-tree-walker#40. It looks like the fix was merged 12 days ago, but there as yet to be a new release cut. This change just fixes Components that use the legacy context API.

@capellini capellini changed the title Fix bug in react-tree-walker call Add context to react-tree-walker call Dec 13, 2018
@isaachinman
Copy link
Contributor

@capellini Thanks for noticing this. I did indeed play around with this previously, as it's related to a dev-only bug which I've hot-fixed with dfa93a8.

Any chance you mind taking a look at that, and seeing if you can spot the problem? I'm 90% sure it's context-related.

@isaachinman isaachinman merged commit ba1c5f1 into i18next:master Dec 14, 2018
@capellini
Copy link
Contributor Author

Any chance you mind taking a look at that, and seeing if you can spot the problem? I'm 90% sure it's context-related.

I took a look at dfa93a8 but it didn't fix the Material UI context problem that I was having. I'm still experiencing problems with areas where I'm using the new Context API, but that's a react-tree-walker known issue, which has a merged PR, but a new release hasn't been cut yet.

But I can say that the problem is most definitely context-related.

@isaachinman
Copy link
Contributor

@capellini I see it's been quite a few months since react-tree-walker released. What do you think about you or I forking, and using that as our dep? We can always revert once a release takes place.

More importantly though, did you try the merged PR and see if it fixes our problem here?

@capellini capellini deleted the react-tree-walker-context branch January 6, 2019 22:45
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