Skip to content

Conversation

knervous
Copy link
Contributor

@knervous knervous commented Oct 1, 2025

Follow-up to the original PR #17175

The different issues pointed out by different people on the BJS forum post point to a caching issue - some scripts were loaded fresh while others were stale and cached by the browser. This led to undefined behavior all around and the page would be effectively "broken" depending on which assets were cached or not, i.e. anything could go wrong and wouldn't necessarily present as a predictable bug.

Previously there were duplicate names from the dist output and monaco worker output which is cached locally in a browser:
image

index.html does not have a Cache-Control response header and respects the client's request of max-age=0 - this is good. index.html should always fetch the latest.

It also appears there is a CDN purge script running on the Playground deployment script, although I don't have access to the CI yaml. https://dev.azure.com/babylonjs/ContinousIntegration/_build/results?buildId=44262&view=logs&j=275f1d19-1bd8-5591-b06b-07d489ea915a&t=4d2ea636-6e6e-5831-8b5a-8ce5e9ce8bbc
If that's the case for the last operation in that script, good. CDN purge handles the server cache.

Takeaway from the existing deployment pipeline and CDN architecture, nothing needs to change. If we can guarantee unique output from a build each time, there is no room for cache confusion from the client's browser. This only applies to the Playground and I haven't looked into other deployed apps. Cache-Control of 2 days is fine. Eventually this issue would've sorted itself out in the field but 2 days is a long time to wait with undefined/broken behavior.

The additional changes in this PR are consolidated in these two commits:

1e9c765
knervous@bd197e8

These changes ensure hashed output from every build and deploy for relevant bundled js files. The generated entrypoint for babylon.playground.[hash].js is injected into the index.html with the WebpackHTMLPlugin. The helper function for fetching scripts does append a query parameter to bust cache, but dependent chunks of scripts do not automatically follow the same convention. Ideally we can get rid of that ?t=[timestamp] and let caching do its business.

For anyone testing changes, please check https://playgroundv2.vercel.app which should be the exact hosted bundle set from the deployment build.

For sanity's sake here are three browsers on mobile tested with the current code: Edge, Firefox and Chrome

image image image

With editor open:

image

@bjsplat
Copy link
Collaborator

bjsplat commented Oct 1, 2025

Please make sure to label your PR with "bug", "new feature" or "breaking change" label(s).
To prevent this PR from going to the changelog marked it with the "skip changelog" label.

@bjsplat
Copy link
Collaborator

bjsplat commented Oct 1, 2025

Reviewer - this PR has made changes to one or more package.json files.

@bjsplat
Copy link
Collaborator

bjsplat commented Oct 1, 2025

@bjsplat
Copy link
Collaborator

bjsplat commented Oct 1, 2025

You have made possible changes to the playground.
You can test the snapshot here:

https://snapshots-cvgtc2eugrd3cgfd.z01.azurefd.net/PLAYGROUND/refs/pull/17216/merge/

The snapshot playground with the CDN snapshot (only when available):

https://snapshots-cvgtc2eugrd3cgfd.z01.azurefd.net/PLAYGROUND/refs/pull/17216/merge/?snapshot=refs/pull/17216/merge

Note that neither Babylon scenes nor textures are uploaded to the snapshot directory, so some playgrounds won't work correctly.

@bjsplat
Copy link
Collaborator

bjsplat commented Oct 1, 2025

You have changed file(s) that made possible changes to the sandbox.
You can test the sandbox snapshot here:

https://snapshots-cvgtc2eugrd3cgfd.z01.azurefd.net/SANDBOX/refs/pull/17216/merge/

@bjsplat
Copy link
Collaborator

bjsplat commented Oct 1, 2025

@bjsplat
Copy link
Collaborator

bjsplat commented Oct 1, 2025

@bjsplat
Copy link
Collaborator

bjsplat commented Oct 1, 2025

@bjsplat
Copy link
Collaborator

bjsplat commented Oct 1, 2025

@sebavan
Copy link
Member

sebavan commented Oct 1, 2025

Amazing thanks a lot, we ll have a dedicated time in the team to test and I ll merge right after and redeploy :-)

@deltakosh
Copy link
Contributor

Bear with us! We are doing a last round of testing on monday and we will merge :D

@knervous
Copy link
Contributor Author

knervous commented Oct 6, 2025

Bear with us! We are doing a last round of testing on monday and we will merge :D

No rush at all, let me know if anything comes up for feedback and will address asap :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants