fix: normalize filled cachebust_origin values in model cache key (#93)#915
Open
leninug wants to merge 1 commit into
Open
fix: normalize filled cachebust_origin values in model cache key (#93)#915leninug wants to merge 1 commit into
leninug wants to merge 1 commit into
Conversation
Extract URL cache-key normalization into a pure `get3DModelCacheKey` utility and route `useGlobalObjLoader` through it. Before this change the cache-busting strip only matched the trailing empty form `&cachebust_origin=`. Real production fixtures (see `stories/assets/complex-board.json`, `nine-key-keyboard.json`) send the filled form `&cachebust_origin=https%3A%2F%2Ftscircuit.com`, which slipped through unchanged. Two boards referencing the same underlying model from different origins produced different cache keys, so the same OBJ was fetched, parsed, and cloned twice — the exact duplicate- load pattern described in tscircuit#93. The new utility handles every position of the parameter (first / middle / last / only-param), with or without a value, and preserves the URL fragment. Wire-up in `useGlobalObjLoader` is a one-line swap. 13 unit tests cover empty / filled / first / middle / trailing / hash-bearing / lookalike-prefix cases plus a real production fixture URL, and assert that two URLs differing only in `cachebust_origin` collapse to the same key. All existing tests continue to pass.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
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
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.
Summary
Fixes a narrow but real duplicate-load case in
useGlobalObjLoaderthat #93 was opened for: URLs that differ only in theircachebust_originquery parameter currently produce different cache keys, so the same OBJ is fetched, parsed, and cloned twice.The existing strip only matched the trailing empty form
&cachebust_origin=. Real production fixtures send the filled form (&cachebust_origin=https%3A%2F%2Ftscircuit.com), which slipped through unchanged. Two boards that reference the same underlying model from different origins hit the cache miss path on every render.Evidence in the repo (no changes to fixtures, just demonstrating the gap):
stories/assets/left-flashlight-board.json— uses the empty form (handled by old regex)stories/assets/complex-board.json,nine-key-keyboard.json— use the filled form (not handled by old regex)Change
New pure utility
src/utils/get-3d-model-cache-key.ts— stripscachebust_originregardless of position (first / middle / last / only-param), with or without a value, and preserves the URL fragment. No dependency onURL(which would throw on relative inputs).useGlobalObjLoaderswaps the inline regex for the utility — one-line wire-up, no behavior change for URLs that don't carrycachebust_origin.New
tests/get-3d-model-cache-key.test.ts— 13 unit tests covering:#fragmentcachebust_origin_extra) — must NOT be strippedcachebust_origin(no=)cachebust_origin→ same keycomplex-board.jsonTest results
Full suite shows the same 5 pre-existing failures before and after this change (all in
preprocess-circuit-json/ SVG snapshot tests, unrelated to the cache layer).Why this is scoped narrowly
The repo already has several recently-merged PRs propagating cache layers to other loaders (GLTF/STL/STEP/WRL). This PR intentionally only touches the OBJ loader path and extracts the normalization into a reusable utility. The utility is exported so other loaders can adopt it incrementally without duplicating the logic.
Algora
/claim #93