Problem/Motivation

The current implementation of drupalSettings updating is kind of a hack that could break easily. Essentially, each Turbo page load inserts a new drupalSettings element because Turbo doesn't know there should only be one of these; the current solution is to remove all but the last one in the DOM during a Turbo render as that's assumed to be the most recent, but this feels like a code smell due to the assumption that the last element will always be the most recent.

Steps to reproduce

👀

Proposed resolution

Implement some explicit method of identifying the drupalSettings for the current response.

This is a bit of a challenge as Turbo seemingly merges in changes to the <head> before it triggers the turbo:before-render event.

We've settled on a solution that decorates the asset.resolver service so that we can alter the output of the AssetResolver::getJsAssets() method to ensure the footer always contains the current page's drupalSettings so there's zero ambiguity and we can find and identify it reliably.

Previous stuff follows, which is partially out of date.


Our MutationObserver implementation fails in certain cases where the observer either does not find a new element at the time it checks, or then finds more than one on subsequent navigations:

  • 4xx errors seem to result in this

These failures are mostly likely due to timing issues since our implementation is a bit of a hack. Unless Turbo adds some API or event we can hook into to be informed of new contents to the <head> being added, we'll have to implement our own solution/workaround.

A short term quick and dirty but reliable solution may be to read the fetch response text and render the response body into a document via DOMParser.parseFromString(), then search the <head> element within that for the new drupalSettings script element of which we can be reasonably sure will only ever be one. That's actually not necessary - see update above.

Ideally, we would implement a server-side solution that allows the front-end JavaScript to reliably receive the drupalSettings for a given response. If Turbo doesn't implement some way to get notified of what <head> changes were made for a given response, we may have to resort to either forcing the drupalSettings <script/> element into the footer or outputting a copy into the footer (which isn't great but gzip should completely negate the network transfer at least) - in both cases this means we can easily identify the correct drupalSettings as it'll be the only one in the <body>.


The subsequent paragraphs are retained for historical purposes but no longer the proposed solution.

One potential solution is to mark the <script> element with an ID and a data-turbo-permanent attribute (see Persisting Elements Across Page Loads) but that would only solve part of the problem because Turbo is probably not going to update the contents, and actually altering that specific element is a bit tricky as it's buried in JsCollectionRenderer::render() so we would probably need to decorate that service and wrap that method. Note that whether the settings element is output to the <head> or in the footer is determined in AssetResolver::getJsAssets() (search for the $settings_in_header variable).

A simpler way to handle this may be to simply use a MutationObserver to watch for any new <script type="application/json" data-drupal-selector="drupal-settings-json"> inserted as direct children of the <head> and remove all other direct children of the <head> matching the script[type="application/json"][data-drupal-selector="drupal-settings-json"] selector. Note that care must be taken to match the full selector and only direct children of the <head> to harden against potential XSS exploits, i.e. an element like this injected into site content or nested in something in the <head> that can include user-entered data such as meta tags, etc.

Remaining tasks

TBD.

User interface changes

None.

API changes

None?

Data model changes

None.

Comments

Ambient.Impact created an issue. See original summary.

ambient.impact’s picture

Issue summary: View changes
ambient.impact’s picture

Assigned: Unassigned » ambient.impact
ambient.impact’s picture

Issue summary: View changes

  • Ambient.Impact committed 83b07ce0 on 2.x
    Issue #3397370: Improved Turbo Improve drupalSettings updating:
    
    Now...
ambient.impact’s picture

Status: Active » Fixed
ambient.impact’s picture

Status: Fixed » Closed (fixed)
ambient.impact’s picture

Assigned: ambient.impact » Unassigned
ambient.impact’s picture

Assigned: Unassigned » ambient.impact
Status: Closed (fixed) » Needs work

Reopening since this still has some edge cases, namely when using the back button, that result in errors in not finding a new Drupal settings element.

  • Ambient.Impact committed 775a9d86 on 2.x
    Issue #3397370: Drupal settings zero or multiple now throw errors:
    
    This...
ambient.impact’s picture

Issue summary: View changes

Reworked proposed resolution with render <head> ourselves solution because the MutationObserver approach still fails in some cases and it's basically a hack anyways.

ambient.impact’s picture

Issue summary: View changes

Expanded short term and long term solutions.

  • Ambient.Impact committed 29e79aa4 on 2.x
    Issue #3397370: Rewrite of drupalSettings for reliability/simplicity:...
ambient.impact’s picture

Issue summary: View changes

Reworked issue summary to include new solution involving decorating the asset.resolver service.

  • Ambient.Impact committed 106c5b0e on 2.x
    Issue #3397370: AssetResolver no longer throw if <body> settings found...
ambient.impact’s picture

ambient.impact’s picture

Assigned: ambient.impact » Unassigned
Status: Needs work » Fixed

Marking as fixed with the most recent commit above.

  • Ambient.Impact committed d1d3e3f0 on 2.x
    Issue #3397370: AssetResolver: merge settings how core does:
    
    I.e. using...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.