Cache OBJ/WRL models globally to prevent duplicate loading

[omwanere]

/claim #93

Fix

• Added global cache for OBJ/WRL model fetch + parse results
• Prevents duplicate parsing when multiple identical components are rendered
• Cached models are cloned before reuse

Demo Video

Attached showing 1 cache miss + 9 cache hits for repeated resistors

Improve.3D.Model.Loading.-.Brave.2026-01-27.17-01-46.mp4

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Review	Updated (UTC)
3d-viewer	Ready	Preview, Comment	Jan 28, 2026 9:45am

[graphite-app]

The nullish coalescing operator ?? won't prevent empty strings. If url is "&cachebust_origin=123", split()[0] returns "" (empty string), and since empty string is not nullish, it won't fall back to url. This results in cleanUrl = "", causing cache misses and fetch failures.

Fix:

const cleanUrl = url.split("&cachebust_origin=")[0] || url

Use || instead of ?? to handle empty strings, or add explicit check:

const parts = url.split("&cachebust_origin=")
const cleanUrl = parts[0] && parts[0].length > 0 ? parts[0] : url

Suggested change

	const cleanUrl = url.split("&cachebust_origin=")[0] ?? url
	const cleanUrl = url.split("&cachebust_origin=")[0] || url

Spotted by Graphite Agent



Is this helpful? React 👍 or 👎 to let us know.

[graphite-app]

Failed loads are cached permanently: When loadAndParseObj() returns an Error, the cache entry remains as { promise, result: null } (set on line 103). All subsequent requests for this URL will return the cached error indefinitely. Transient failures (network issues, temporary server errors) can never recover without a page refresh.

// Fix: Don't cache errors, or add cache expiry
const promise = loadAndParseObj().then((result) => {
  if (!(result instanceof Error)) {
    cache.set(cleanUrl, { promise, result })
  } else {
    // Remove failed entry to allow retry
    cache.delete(cleanUrl)
  }
  return result
})

Suggested change

	if (!(result instanceof Error)) {
	cache.set(cleanUrl, { promise, result })
	}
	if (!(result instanceof Error)) {
	cache.set(cleanUrl, { promise, result })
	} else {
	// Remove failed entry to allow retry
	cache.delete(cleanUrl)
	}

Spotted by Graphite Agent



Is this helpful? React 👍 or 👎 to let us know.

This comment came from an experimental review—please leave feedback if it was helpful/unhelpful. Learn more about experimental comments here.

[Copilot]

Pull request overview

This PR introduces global caching for 3D model loading to avoid repeatedly fetching and parsing the same OBJ/WRL/STL/GLTF assets, addressing performance issues described in Issue #93 for circuits that reuse identical models.

Changes:

• Added a global modelCache around load3DModel to cache parsed STL/OBJ/WRL/GLTF models and return clone()d instances for each consumer.
• Adjusted the WRL loading path to use the shared loadVrml helper while integrating it into the new cache.
• Updated useGlobalObjLoader to be more robust (SSR-safe guard, simplified cachebust handling, and cleaner cache access) while preserving its global OBJ/WRL cache semantics.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
src/utils/load-model.ts	Adds a global modelCache and clones cached STL/OBJ/WRL/GLTF models on reuse to avoid redundant network and parse work when components share the same model_*_url.
src/hooks/use-global-obj-loader.ts	Refines the global OBJ/WRL hook by normalizing URLs (removing cachebust_origin), guarding for non-browser environments, and simplifying cache result handling and effect cleanup.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

modelCache is a process-wide Map with no eviction, so every distinct URL loaded via load3DModel will be retained for the lifetime of the page, which may increase memory usage over time in long-running sessions that touch many different models. If this becomes a concern, consider adding a way to clear the cache or bounding its size (e.g., LRU), in line with how you want caching to behave elsewhere in the app.

[Copilot]

loadVrml is declared to always either return an Object3D or throw (see src/utils/vrml.ts:19-29), so the if (model) { ... } check and the return null in this WRL branch are effectively dead code. To keep this helper easier to reason about, consider removing the null-check and unreachable return null so WRL behaves like the other loaders (returning a model on success and rejecting on failure).

Suggested change

	if (model) {
	modelCache.set(url, model)
	return model.clone()
	}
	return null
	modelCache.set(url, model)
	return model.clone()

[rushabhcodes]

Also you can join our discord server to get your pr review faster https://discord.com/invite/V7FGE5ZCbA

[graphite-app]

Race condition: Multiple simultaneous requests for the same uncached URL will all trigger separate network requests and parsing operations. Unlike use-global-obj-loader.ts which caches the promise itself, this only checks for completed results.

What breaks: If 10 components mount simultaneously requesting the same model, all 10 will make separate network requests instead of sharing one.

Fix: Cache the loading promise, not just the result:

const modelCache = new Map<string, THREE.Object3D>()
const loadingPromises = new Map<string, Promise<THREE.Object3D | null>>()

export async function load3DModel(url: string): Promise<THREE.Object3D | null> {
  if (modelCache.has(url)) {
    return modelCache.get(url)!.clone()
  }
  
  if (loadingPromises.has(url)) {
    const result = await loadingPromises.get(url)!
    return result ? result.clone() : null
  }
  
  const promise = (async () => {
    // ... existing loading logic ...
  })()
  
  loadingPromises.set(url, promise)
  const result = await promise
  loadingPromises.delete(url)
  
  return result ? result.clone() : null
}

Spotted by Graphite Agent



Is this helpful? React 👍 or 👎 to let us know.

This comment came from an experimental review—please leave feedback if it was helpful/unhelpful. Learn more about experimental comments here.

[omwanere]

Resolved

[omwanere]

Any update on my PR?