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

Added support for @hotwired/turbo. #1374

Merged
merged 5 commits into from
Jun 29, 2021
Merged

Conversation

pgruener
Copy link
Contributor

@pgruener pgruener commented May 18, 2021

This change is Reviewable

if (context.ReactOnRails) {
return context.ReactOnRails.option('turbo') === true;
}
}
Copy link
Member

Choose a reason for hiding this comment

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

Is a configuration better than detection?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, I also would prefer a detection if possible. But as I noticed Turbo will be included only as module (for webpack(er)) in contrast to Turbolinks, which will inject itself to the global window.

So I don't know a way to inspect the global environment to detect if the Turbo module is used somewhere in the application.. if you have a hint for me, we can adjust that of course.

Copy link
Member

Choose a reason for hiding this comment

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

Try require within a try block.

I can get you a cod snippet if needed.

Copy link
Contributor

Choose a reason for hiding this comment

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

The problem with detection logic like that is that it would only detect if turbo is being bundled through webpacker even though turbo supports sprockets as well.

@justin808
Copy link
Member

@pgruener consider joining our Slack

@justin808
Copy link
Member

@pgruener Let me know if I should take over your PR or if you have an ETA. I'd like to ship it in the next release.

@pgruener
Copy link
Contributor Author

pgruener commented Jun 1, 2021

@justin808 Thanks for your message. You can of course take over, as I didn't get time yet to go on with your suggestion (even if it should only take a few minutes). My best ETA would be around the coming weekend, but I'd really appreciate it to be shipped soon.
I also did join the slack channel, but didn't get a chance to get active there.

@justin808
Copy link
Member

@pgruener Say 👋 when you get to the Slack. Next weekend would be fine. I've got a ton of other pending tasks. I really appreciate the help!

@justin808
Copy link
Member

@pgruener Any update?

@justin808
Copy link
Member

@Judahmeek can you review also?

@justin808
Copy link
Member

@pgruener can you see if you can fix the CI issues?

@justin808
Copy link
Member

We need some short docs on this. Maybe we should update this page: https://github.com/shakacode/react_on_rails/blob/9ef6b326213480d0022db42710e5517cad0317bc/docs/rails/turbolinks.md ?

or is turbo different enough that we should create a new doc page?

@justin808 justin808 merged commit 4e9570b into shakacode:master Jun 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.

3 participants