JS error thrown when Turbo-rails initialized twice on 4xx/5xx

[clinejj]

We're seeing the following error

Uncaught DOMException: Failed to execute 'define' on 'CustomElementRegistry': the name "turbo-frame" has already been used with this registry

from these lines: https://github.com/hotwired/turbo-rails/blob/f3293d256d20c6436ab3b819c9d5eb96b79b5c64/app/assets/javascripts/turbo.js#L3229-L3233

Related to https://github.com/hotwired/turbo/issues/188, this is happening because using a custom error handler using the method here: https://web-crunch.com/posts/custom-error-page-ruby-on-rails, basically:

• config.exceptions_app = self.routes
• Update routes to send 4xx and 5xx requests to custom ErrorsController
• ErrorsController renders a view which uses our normal application layout, which includes the Turbo initialization

turbo-rails is imported before Turbo, so while Turbo looks to protect itself from an attempt to start twice, turbo-rails does not because that code gets executed first.

It's not clear the best way to handle this case if you want to reuse your page layout rather than having a hardcoded 4xx/5xx page. Turbo uses the ErrorRenderer for 4xx/5xx responses, which replaces both the body and head, rather than just the body that PageRenderer uses, which leads to the head being replaced and reinitialized rather than just the body.

[seanabrahams]

Digging into this a little bit it looks like https://github.com/hotwired/turbo/pull/483 is on to something. In addition to those conditionals, turbo-rails https://github.com/hotwired/turbo-rails/blob/6f6d8c7cfb5f18fb7b71b47c7e73691b9e7d7e2e/app/javascript/turbo/cable_stream_source_element.js#L27 needs to become:

if (customElements.get("turbo-cable-stream-source") === undefined) {
  customElements.define("turbo-cable-stream-source", TurboCableStreamSourceElement)
}

With all of these conditionals in place no JS errors are encountered, however I'm not yet familiar enough with the libraries to know what the downsides are with this path. Perhaps it's better to be made aware of when this case is encountered as it's a code smell, but perhaps it's not avoidable in some circumstances.

[jdelStrother]

@seanabrahams Seems like it would be better to change things in Turbo so that error responses don't blindly replace the head & body, instead doing the same snapshot comparisons to decide which new script elements need adding. One example problem I'm seeing with the current behaviour is that we use Fontawesome icons which when first loaded, inject style tags into the head on our regular pages. Then if you later receive a error response, Turbo deletes the head and body which causes our error page's icons to be missing styles.

Seems like ErrorRenderer ignores turbo-visit-control=reload, too, which makes this quite tricky to fix cleanly.

It does seem to behave ok if I naively hack the source -

diff --git a/src/core/drive/visit.ts b/src/core/drive/visit.ts
--- src/core/drive/visit.ts
+++ src/core/drive/visit.ts
@@ -262,9 +262,9 @@
           this.performScroll()
           this.adapter.visitRendered(this)
           this.complete()
         } else {
-          await this.view.renderError(PageSnapshot.fromHTMLString(responseHTML), this)
+          await this.view.renderPage(PageSnapshot.fromHTMLString(responseHTML), false, this.willRender, this)
           this.adapter.visitRendered(this)
           this.fail()
         }
       })

but I can see that it might need more thought ... maybe error responses need to opt into this behaviour (eg in header or metatags), so that unsuspecting pages still get a full page replacement (eg for gateway errors from a service that knows nothing of turbo)

[n-rodriguez]

Hi there! Any news?

[n-rodriguez]

In the meantime, for those who don't know (like me, until then) you can use : https://yarnpkg.com/cli/patch

yarn patch @hotwired/turbo
subl /private/var/folders/90/31t09ry509sdf_8vy_psjm5w0000gn/T/xfs-35500cf6/user
yarn patch-commit -s /private/var/folders/90/31t09ry509sdf_8vy_psjm5w0000gn/T/xfs-35500cf6/user
yarn install

Be sure to patch the right file (dist/turbo.es2017-esm.js) or you'll spend hours asking yourself "why the patch don't apply" (I'm using Webpack with Shakapacker)