Overview

When upgrading XB between alpha releases, pages can appear broken. A hard refresh is required by browsers to bypass browser cache for the bundled assets and imports.

Proposed resolution

Load bundle.js and friends with a cache buster query string.

Per Matt's note, we aren't using AssetQueryStringInterface, but the XB version read from the ui/package.json.

In cases where that version is exposed to the public, we use a hashed version of it.

User interface changes

Better front-end performance with proper cache busting version query.

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

mglaman created an issue. See original summary.

lauriii’s picture

Priority: Normal » Critical
Issue tags: +beta blocker
penyaskito’s picture

Assigned: Unassigned » penyaskito
balintbrews’s picture

So we have the items in the import map generated in \Drupal\experience_builder\Plugin\ExperienceBuilder\ComponentSource\JsComponent::renderComponent.

Then we have a lot of JS files included in libraries (i.e. in experience_builder.libraries.yml) where the version gets appended from the library definition, but we never update those, and it's not even feasible for xb-ui as that gets recompiled and updated constantly. How are those usually handled? I think I've seen it before that a hash was added to the filename by the JS bundler, and the filename was read in hook_library_info_alter().

penyaskito’s picture

Component: … to be triaged » Page builder

(trying to categorize this, not sure what is the best match, assuming Page builder)


If I understood this, most of the issues come from js components?
Adding a cache buster query there from AssetQueryStringInterface.
Wondering if we could use the component version though.


Then we have a lot of JS files included in libraries (i.e. in experience_builder.libraries.yml) where the version gets appended from the library definition, but we never update those, and it's not even feasible for xb-ui as that gets recompiled and updated constantly. How are those usually handled? I think I've seen it before that a hash was added to the filename by the JS bundler, and the filename was read in hook_library_info_alter().

I see 2 options here:

1) KISS, just remove version, and Drupal will handle it using the same AssetQueryStringInterface that uses for the rest of assets.
2) hook_library_info_alter reading ui/package.json, which we should keep in sync on release numbers.

penyaskito’s picture

Assigned: penyaskito » larowlan
Status: Active » Needs review

Needs feedback on direction.

larowlan’s picture

Assigned: larowlan » penyaskito

+1 for keeping it simple in the first instance

larowlan’s picture

FWIW we could also configure vite to include hash in filenames https://rollupjs.org/configuration-options/#output-entryfilenames
That's something we do on client projects

penyaskito’s picture

Assigned: penyaskito » larowlan
Issue summary: View changes

We now use the package.json version (or a hashed variation of that).
Back to Lee for review in case he has the time, I will pass this by Bálint tomorrow (our) morning.

penyaskito’s picture

Assigned: larowlan » balintbrews
wim leers’s picture

Component: Page builder » Component sources

(trying to categorize this, not sure what is the best match, assuming Page builder)

Appreciate that! 😊

But it should be Component sources, since this is a problem only for the js ComponentSource plugin ("code components").

wim leers’s picture

Assigned: balintbrews » penyaskito
Status: Needs review » Needs work
wim leers’s picture

penyaskito’s picture

Assigned: penyaskito » wim leers
Status: Needs work » Needs review

Ended up implementing a MockVersion for testing purposes. All feedback addressed.
Crediting effulgentsia for feedback on MR.

balintbrews’s picture

Status: Needs review » Needs work

This looks really great!

I think long term we should consider what both @larowlan and I suggested (in #9 and #4), having Vite/Rollup output filenames with a hash in them. That would allow us to do releases with backend-only changes where we don't force re-downloading unchanged JavaScript assets.

balintbrews’s picture

Status: Needs work » Needs review

Oops, didn't mean to do that.

wim leers’s picture

Assigned: wim leers » penyaskito
Status: Needs review » Needs work
Issue tags: +Needs followup

#17++ — but we'll still need what this MR does for the xb-ui asset library's definition itself. Tagging Needs followup for this 👍

So very close!

wim leers’s picture

Title: Provide cache buster query string on imports » Provide cache buster query string for imports + UI

Reflecting the slightly expanded scope per @balintbrews and my request 😇

penyaskito’s picture

Assigned: penyaskito » wim leers
Status: Needs work » Needs review

Now we fallback to AssetQueryString also in xb-ui and astro libs.


#17, #19: That might make harder to include the assets as we do right now, and specially would make tests more complicated? Not sure how worth would it be, and it would differ from what core does.

wim leers’s picture

Assigned: wim leers » Unassigned
Status: Needs review » Reviewed & tested by the community

LGTM — and many thanks for the ℹ️ comments on the MR, @penyaskito!

Just added some clarifying comments based on the discussion on the issue + MR and fixed some language nits — 🚢

  • wim leers committed 648cb639 on 0.x authored by penyaskito
    Issue #3536093 by penyaskito, wim leers, balintbrews, larowlan, mglaman...
wim leers’s picture

Status: Reviewed & tested by the community » Patch (to be ported)

🥳

Still needs follow-up issue for #4 + #9 + #17, which are all about the same thing.

mayur-sose’s picture

ID Steps Expected Result Pass/Fail
TC1
  1. Add a new page with a code component.
  2. Open the page in the browser.
  3. Open DevTools Network tab.
  4. Inspect all asset URLs (JS, CSS, etc.).
All asset URLs have a query string (e.g., ?something).

Exception: Files with auto-generated JS names may not have it.
Pass
TC2
  1. Open the same page for editing in Experience Builder.
  2. Open DevTools Network tab.
  3. Find /ui/dist/assets/index.js in the requests.
The URL for /ui/dist/assets/index.js includes a query string (e.g., ?something). Pass
TC3
  1. In the ui folder, run npm version 4.1.3 to fake a new version.
  2. Clear Drupal cache.
  3. Add a new page with a code component.
  4. Open and inspect assets in DevTools.
Asset URLs have query strings.

The value for the query string is different than before.
Pass
TC4
  1. On page with code component, check all asset names in Network tab.
  2. Look for assets named like astro-*.
Each astro-* asset URL ends with ?4.1.3. Pass
TC5
  1. In Experience Builder (edit mode), open Network tab.
  2. Look for ui/dist/index.js and astro-* assets.
Both ui/dist/index.js and astro-* assets have ?4.1.3 as their query strings in the network panel. Pass
TC6
  1. While still in XB edit mode, look for all non-astro asset requests.
  2. Compare their query string to before version bump.
All non-astro assets have a different query string than before, reflecting the library version change. Pass
wim leers’s picture

Thanks, @mayur-sose!

penyaskito’s picture

Status: Patch (to be ported) » Fixed
Issue tags: -Needs followup

Status: Fixed » Closed (fixed)

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