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

Defensively create custom turbo elements #483

Merged
merged 1 commit into from
Jun 19, 2022

Conversation

dphaener
Copy link
Contributor

There are instances when turbo attempts to create the frame and stream
elements when they have already been registered. This is easily
reproducible in development when using webpacker by following these steps:

  • Make a change to any file that webpacker compiles.
  • Click a turbo enabled link.
  • Boom.

When Turbo replaces the page it appears as if it reloads the Turbo
library and attempts to create the custom elements again. I have also
observed this behavior in my production app via my exception monitoring
service, but have been unable to reproduce this behavior in production
on my own.

This change simply checks for the existence of the custom element prior
to to attempting to define it. The get method used here explicitly
returns undefined when the element does not yet exist so it is safe to
use the strict equality operator here. Reference:
https://developer.mozilla.org/en-US/docs/Web/API/CustomElementRegistry/get

Fixes #188

There are instances when turbo attempts to create the frame and stream
elements when they have already been registered. This is easily
reproducible in development when using webpacker by following these steps:

* Make a change to any file that webpacker compiles.
* Click a turbo enabled link.
* Boom.

When Turbo replaces the page it appears as if it reloads the Turbo
library and attempts to create the custom elements again. I have also
observed this behavior in my production app via my exception monitoring
service, but have been unable to reproduce this behavior in production
on my own.

This change simply checks for the existence of the custom element prior
to to attempting to define it. The `get` method used here explicitly
returns `undefined` when the element does not yet exist so it is safe to
use the strict equality operator here. Reference:
https://developer.mozilla.org/en-US/docs/Web/API/CustomElementRegistry/get
@dphaener
Copy link
Contributor Author

dphaener commented Dec 1, 2021

Update: This happens reliably in production when the webpack bundle has changed (after a deploy) and a turbo enabled link is clicked.

@dphaener
Copy link
Contributor Author

dphaener commented Feb 8, 2022

Yet another update: After updating my application to use jsbundling-rails and esbuild I am no longer seeing this error on deploys (or at all for that matter).

@t27duck
Copy link
Contributor

t27duck commented Feb 8, 2022

Is that with or without this patch with your update to jsbundling-rails?

@dphaener
Copy link
Contributor Author

dphaener commented Feb 8, 2022

@t27duck Without the patch. I honestly don't know why. 🤷

@mbardelmeijer
Copy link

We can also reproduce this in production where on deploy, the bundle changes and this error is triggered after clicking on any turbo link. Would love to see this merged.

@denydias denydias mentioned this pull request Apr 19, 2022
@dhh dhh merged commit d0125a5 into hotwired:main Jun 19, 2022
dhh added a commit to feliperaul/turbo that referenced this pull request Jul 16, 2022
* main:
  Allow frames to scroll smoothly into view (hotwired#607)
  Export Type declarations for `turbo:` events (hotwired#452)
  Add .php as a valid isHTML extension (hotwired#629)
  Add original click event to 'turbo:click' details (hotwired#611)
  Drive Browser tests with `playwright` (hotwired#609)
  Allow Turbo Streams w/ GET via `data-turbo-stream` (hotwired#612)
  Only update history when Turbo visit is renderable (hotwired#601)
  Support development ChromeDriver version overrides (hotwired#606)
  Turbo stream source (hotwired#415)
  Expose Frame load state via `[complete]` attribute (hotwired#487)
  fix(ie/edge): form.method='delete', raises Invalid argument. (hotwired#586)
  Do not declare global types/constants (hotwired#524)
  Defensively create custom turbo elements (hotwired#483)
  Use `replaceChildren` in StreamActions.update (hotwired#534)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

404 causes DOMException
4 participants