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

Compatibility with React 15.4 #11

Closed
mjomble opened this issue Nov 16, 2016 · 6 comments
Closed

Compatibility with React 15.4 #11

mjomble opened this issue Nov 16, 2016 · 6 comments

Comments

@mjomble
Copy link
Contributor

mjomble commented Nov 16, 2016

This line in index.js fails with React 15.4.0:

var SyntheticKeyboardEvent = require('react/lib/SyntheticKeyboardEvent');

From the React 15.4.0 blog post:

If you only use the official and documented React APIs you won't need to change anything in your application.
However, there is a possibility that you imported private APIs from react/lib/*, or that a package you rely on might use them. We would like to remind you that this was never supported, and that your apps should not rely on internal APIs. The React internals will keep changing as we work to make React better.

@glenjamin
Copy link
Owner

I'm not actively using this module at the moment, so unlikely to have time to delve into this.

I'll happily take a PR though.

One of the original goals with this lib was to get a React-wrapped event object rather than the standard browser event object - which is why I had to reach into the internals. This might not be as important these days.

@hotohoto
Copy link
Contributor

hotohoto commented Nov 17, 2016

It may only require to replace
require('react/lib/SyntheticKeyboardEvent');
with
require('react-dom/lib/SyntheticKeyboardEvent');

@hotohoto
Copy link
Contributor

What is the reason why my PR have been failed?

https://travis-ci.org/glenjamin/react-hotkey/builds/176617307

@glenjamin
Copy link
Owner

Should be resolved in 0.7.0

@mjomble
Copy link
Contributor Author

mjomble commented Nov 17, 2016

This will do as a quick fix, but there is absolutely no guarantee that a future change to React internals won't unexpectedly break it again.

It might be a good idea to look into a more permanent solution or at least update the README to inform users of this library that it relies on an unstable technique and may unexpectedly break builds.

I can provide a PR for the README if needed.

@glenjamin
Copy link
Owner

PR for the readme to warn about internals usage sounds good.

React doesn't expose the synthetic events as a supported API, so there's no better way to get the same effect. See facebook/react#285 for more details about this.

I have some vague ideas about a different way to approach hotkeys, using a child component and mousetrap, but nothing I'm likely to work on or publish any time soon.

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

No branches or pull requests

3 participants