Defensively create custom turbo elements#483
Merged
dhh merged 1 commit intohotwired:mainfrom Jun 19, 2022
Merged
Conversation
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
Contributor
Author
|
Update: This happens reliably in production when the webpack bundle has changed (after a deploy) and a turbo enabled link is clicked. |
Contributor
Author
|
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). |
Contributor
|
Is that with or without this patch with your update to jsbundling-rails? |
Contributor
Author
|
@t27duck Without the patch. I honestly don't know why. 🤷 |
|
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. |
Closed
dhh
pushed 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)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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:
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
getmethod used here explicitlyreturns
undefinedwhen the element does not yet exist so it is safe touse the strict equality operator here. Reference:
https://developer.mozilla.org/en-US/docs/Web/API/CustomElementRegistry/get
Fixes #188