Skip to content

Resolve double mounting error with Turbo(links) #1421

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.

Sign up for GitHub

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

Closed
wants to merge 6 commits into from
Closed

Resolve double mounting error with Turbo(links) #1421

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
29ebe20
enhance turbo documentation
Judahmeek Jan 14, 2022
467b032
switch from render to load event listeners
Judahmeek Jan 14, 2022
a6cc1e2
changelog
Judahmeek Jan 14, 2022
5a1b748
update setOptions typing
Judahmeek Jan 17, 2022
8cd4b57
restrict generator mini-css-extract-plugin version to 2.4.5
Judahmeek Jan 17, 2022
0d94ca7
add typescripts changes
Judahmeek Jan 18, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@ Please follow the recommendations outlined at [keepachangelog.com](http://keepac
### [Unreleased]
Changes since last non-beta release.

### Fixed
- Double rendering of components with using Turbo or Turbolinks. [PR 1421](https://github.com/shakacode/react_on_rails/pull/1421) by [judahmeek](https://github.com/judahmeek).

*Please add entries here for your pull requests that are not yet released.*
### [12.5.2] - 2021-12-29
### Fixed
Expand Down
3 changes: 2 additions & 1 deletion docs/api/javascript-api.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,8 @@ The best source of docs is the main [ReactOnRails.js](https://github.com/shakaco
/**
* Set options for ReactOnRails, typically before you call ReactOnRails.register
* Available Options:
* `traceTurbolinks: true|false Gives you debugging messages on Turbolinks events
* `traceTurbolinks: true|false` Gives you debugging messages on Turbolinks events
* `turbo: true|false` enables Turbo, which is not auto-detected like the Turbolinks library is
Copy link

Choose a reason for hiding this comment

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

@Judahmeek do we need to update this here as it doesn't seem to include turbo?

Screen Shot 2022-01-14 at 3 39 50 PM

Copy link
Member

Choose a reason for hiding this comment

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

we could allow for either string, but it's not totally required.

Copy link

@templeman15 templeman15 Jan 15, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Yeah I was thinking turbo?: boolean

Or turbo: boolean | null or however it's actually defined.

I think it'd help increase visibility because my IDE only shows traceTurboLinks.

I'm speaking specifically about the d.ts file.

Copy link
Member

Choose a reason for hiding this comment

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

if we could easily just have the shorter traceTurbo, that's better for the long-term.

Copy link
Member

Choose a reason for hiding this comment

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

@Judahmeek, yes, we need to update the d.ts file.

Copy link

Choose a reason for hiding this comment

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

@justin808 my interpretation of the code seems slightly different.

For Turbo to work I need:

turbo: true

To see the logging I need:

traceTurboLinks: true

Both have to be on for it to work and log with Turbo.

--

Turbolinks seems to work without opting into it. traceTurboLinks: true just prints to the logs in that scenario.

*/
setOptions(options)

Expand Down
2 changes: 1 addition & 1 deletion lib/generators/react_on_rails/base_generator.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ def add_yarn_dependencies
babel-plugin-macros"

puts "Adding CSS handlers"
run "yarn add css-loader css-minimizer-webpack-plugin mini-css-extract-plugin style-loader"
run "yarn add css-loader css-minimizer-webpack-plugin mini-css-extract-plugin@2.4.5 style-loader"

puts "Adding dev dependencies"
run "yarn add -D @pmmmwh/react-refresh-webpack-plugin react-refresh"
Expand Down
12 changes: 6 additions & 6 deletions node_package/src/clientStartup.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -241,16 +241,16 @@ function renderInit(): void {
if (turboInstalled()) {
debugTurbolinks(
'USING TURBO: document added event listeners ' +
'turbo:before-render and turbo:render.');
document.addEventListener('turbo:before-render', reactOnRailsPageUnloaded);
document.addEventListener('turbo:render', reactOnRailsPageLoaded);
'turbo:before-visit and turbo:load.');
document.addEventListener('turbo:before-visit', reactOnRailsPageUnloaded);
document.addEventListener('turbo:load', reactOnRailsPageLoaded);
reactOnRailsPageLoaded();
} else if (turbolinksVersion5()) {
debugTurbolinks(
'USING TURBOLINKS 5: document added event listeners ' +
'turbolinks:before-render and turbolinks:render.');
document.addEventListener('turbolinks:before-render', reactOnRailsPageUnloaded);
document.addEventListener('turbolinks:render', reactOnRailsPageLoaded);
'turbolinks:before-visit and turbolinks:load.');
document.addEventListener('turbolinks:before-visit', reactOnRailsPageUnloaded);
document.addEventListener('turbolinks:load', reactOnRailsPageLoaded);
Copy link
Member

Choose a reason for hiding this comment

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

@Judahmeek did you test on Turbolinks 5?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I did.

reactOnRailsPageLoaded();
} else {
debugTurbolinks(
Expand Down
2 changes: 1 addition & 1 deletion node_package/src/types/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ export interface ReactOnRails {
register(components: { [id: string]: ReactComponentOrRenderFunction }): void;
registerStore(stores: { [id: string]: Store }): void;
getStore(name: string, throwIfMissing: boolean): Store | undefined;
setOptions(newOptions: {traceTurbolinks: boolean}): void;
setOptions(newOptions: {traceTurbolinks?: boolean, turbo?: boolean}): void;
reactOnRailsPageLoaded(): void;
authenticityToken(): string | null;
authenticityHeaders(otherHeaders: { [id: string]: string }): AuthenticityHeaders;
Expand Down