Only use turbolinks events if supported #93
Conversation
The current code always uses Turbolinks events if it is loaded, but this doesn't mean it is supported by the browser. This change checks if turbolinks is supported and otherwise uses the native event handlers. Without this fix ujs components are not rendered in IE8.
|
Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla - and if you have received this in error or have any questions, please drop us a line at [email protected]. Thanks! |
|
I've signed the CLA, not sure why the bot isn't posting that. I've mailed [email protected] for clarification. |
|
👏 👏 thank you for finding this and fixing it! 👍 |
|
Btw. this also seems to be fixing a checksum error I was seeing with server side rendering in IE8. Before this change I was getting the following error when refreshing a pre-rendered component using js:
|
|
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks! |
|
@felixbuenemann should we have any tests here? |
|
@JakubMal What would you test? You could stub out Turbolinks.supported to simulate an unsupported browser, but I'm not sure that would be that useful. |
|
Probably fine without tests. I'll leave it to you @JakubMal |
|
This is an IE8 regression, so how about we create Sauce Labs account for cases like this? (this way we can automatically test browsers we want to support - probably the same browsers React support. |
|
👍 this is a definite improvement, right? browser testing would be nice but does this have to be blocked in the meantime? |
…handler-fix Only use turbolinks events if supported
|
Thanks, @rmosolgo! |
The current code always uses Turbolinks events if it is loaded, but this
doesn't mean it is supported by the browser. This change checks if
turbolinks is supported and otherwise uses the native event handlers.
Without this fix ujs components are not rendered in IE8.