Coalesce concurrent requests for the same schema - #242
Conversation
✅ Deploy Preview for gobl-build ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
The SchemaRegistry only caches a schema once its fetch resolves, so consumers asking for the same schema while a request was still in flight — parallel branches of the form parser plus the Monaco preload at mount time — each missed the cache and issued their own fetch, loading every schema two or three times. fetchExternalSchema now tracks in-flight requests by URL and shares the pending promise between callers, so each schema is fetched exactly once. loadSchemaSet's strict root fetch joins the same map, keeping its throw-on-failure semantics. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1650451 to
f95f5a6
Compare
There was a problem hiding this comment.
Pull request overview
This PR reduces duplicate JSON Schema network requests during editor mount by coalescing concurrent fetches for the same schema URL in the form schema loader, so parallel consumers share a single in-flight request.
Changes:
- Add an in-flight promise cache (
pendingSchemas) and a strict fetch helper to dedupe concurrent schema loads and preserve throw-on-root-load behavior. - Route
loadSchemaSetroot schema loading through the same strict/coalesced fetch path. - Adjust editor
apiBaseUrlprop defaults/docs so nested editors don’t reset an embedder-configured endpoint.
Reviewed changes
Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/lib/editor/form/utils/schema.ts | Adds in-flight request coalescing and uses strict root fetching for schema preloads. |
| src/lib/types/editor.ts | Updates apiBaseUrl prop documentation to reflect “inherit existing endpoint” behavior. |
| src/lib/ObjectEditor.svelte | Stops defaulting apiBaseUrl to the global default to avoid nested editor resets. |
| src/lib/EnvelopeEditor.svelte | Same apiBaseUrl default/inheritance change as ObjectEditor. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
src/lib/editor/form/utils/schema.ts:46
pendingSchemasis a plain object keyed by arbitrary schema IDs/URLs. Using a normal object can lead to key-collision/prototype-pollution edge cases (e.g.__proto__,constructor) if a schema reference ever contains such a value. AMapavoids these hazards and is a better fit for an in-flight cache.
const pendingSchemas: Record<string, Promise<Schema>> = {}
function fetchExternalSchemaStrict(id: string): Promise<Schema> {
const schema = SchemaRegistry[id]
if (schema) return Promise.resolve(schema)
The apiBaseUrl prop defaulted to the public gobl.dev service, so any editor mounted without an explicit endpoint — such as a nested correct or headers modal — reset the shared client back to the default, undoing the embedder's configuration and sending stray requests to gobl.dev. Default the prop to an empty string instead, which setApiBaseUrl treats as a no-op, so editors without an explicit endpoint inherit whatever is already configured. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
src/lib/editor/form/utils/schema.ts:46
pendingSchemasis a plain object keyed by URL. Since these keys ultimately come from external schema URLs /$refvalues, using aRecordrisks special keys like__proto__/constructorcausing prototype pollution and hard-to-debug behavior. Use aMapfor the in-flight cache to avoid property-key hazards.
const pendingSchemas: Record<string, Promise<Schema>> = {}
function fetchExternalSchemaStrict(id: string): Promise<Schema> {
const schema = SchemaRegistry[id]
if (schema) return Promise.resolve(schema)
Summary
Fixes schemas being fetched two or three times on editor mount, plus a related endpoint-reset issue. Two commits:
Coalesce concurrent requests for the same schema
The
SchemaRegistryonly caches a schema after its fetch resolves, so concurrent consumers — parallel branches of the form parser'sparseSchemarecursion plus the MonacoloadSchemaSetpreload, all racing at mount — each missed the cache and issued their own request for the same URL.fetchExternalSchemanow keeps an in-flight map keyed by URL: the first caller starts the fetch and stores the promise, later callers await the same promise, and the entry is cleared on settle (so a failed fetch can be retried rather than caching the failure).loadSchemaSet's strict root fetch goes through the same map via a newfetchExternalSchemaStrict, preserving its throw-on-failure semantics while joining the coalescing.The custom JSON worker's
schemaRequestServiceis unaffected: it only runs in theenableSchemaRequest: truefallback, which the preloading path disables.Keep the configured API endpoint when editors mount without one
apiBaseUrldefaulted to the public gobl.dev service, so any editor mounted without an explicit endpoint (e.g. a nested correct/headers modal) reset the shared client back to the default, undoing the embedder's configuration and sending stray requests to gobl.dev. The prop now defaults to an empty string, whichsetApiBaseUrltreats as a no-op, so editors without an explicit endpoint inherit whatever is already configured.Testing
npm run check— 0 errors, 0 warnings.npm run lintandnpm run build:packageclean.🤖 Generated with Claude Code