From 69d390a3852a8ae86f7bc589b985acd60dadd46c Mon Sep 17 00:00:00 2001 From: Luke Melia Date: Mon, 17 Aug 2026 18:54:09 -0400 Subject: [PATCH 1/2] Give every FileDef subtype module a default export; name the field in thunk errors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The subtype family had two export conventions: the shell-based modules export their def class both named and default, while 17 older leaf modules (text/image/audio families) were named-only. A default import of a named-only module evaluates to undefined, which then fails far from the import at schema time with an error suggesting a cyclic dependency — naming neither the field nor the cause. All subtype modules now export their primary class both ways. cardThunk now receives the field name and owning prototype from every field initializer (contains/containsMany/linksTo/linksToMany), so an undefined field card class throws an error naming the exact field and card and describing both real causes: an import that doesn't match the module's export shape, and a genuine cycle needing the thunk form. FILEDEF_CODE_REF_BY_EXTENSION is exported so the new export-shape test can walk the registry and stay current as subtypes are added. Co-Authored-By: Claude Fable 5 --- packages/base/avif-image-def.gts | 2 + packages/base/card-api.gts | 35 ++++++++---- packages/base/csv-file-def.gts | 2 + packages/base/flac-audio-def.gts | 2 + packages/base/gif-image-def.gts | 2 + packages/base/gts-file-def.gts | 2 + packages/base/jpg-image-def.gts | 2 + packages/base/json-file-def.gts | 2 + packages/base/m4a-audio-def.gts | 2 + packages/base/markdown-file-def.gts | 2 + packages/base/mp3-audio-def.gts | 2 + packages/base/ogg-audio-def.gts | 2 + packages/base/png-image-def.gts | 2 + packages/base/svg-image-def.gts | 4 +- packages/base/text-file-def.gts | 2 + packages/base/ts-file-def.gts | 2 + packages/base/wav-audio-def.gts | 2 + packages/base/webp-image-def.gts | 2 + .../host/tests/unit/card-thunk-error-test.ts | 36 +++++++++++++ .../tests/unit/filedef-export-shape-test.ts | 53 +++++++++++++++++++ packages/runtime-common/file-def-code-ref.ts | 2 +- 21 files changed, 151 insertions(+), 11 deletions(-) create mode 100644 packages/host/tests/unit/card-thunk-error-test.ts create mode 100644 packages/host/tests/unit/filedef-export-shape-test.ts diff --git a/packages/base/avif-image-def.gts b/packages/base/avif-image-def.gts index 819f570d7f4..7f7f35d48a4 100644 --- a/packages/base/avif-image-def.gts +++ b/packages/base/avif-image-def.gts @@ -39,3 +39,5 @@ export class AvifDef extends RasterImageDef { }; } } + +export default AvifDef; diff --git a/packages/base/card-api.gts b/packages/base/card-api.gts index 2524d863f88..831a53ccfe9 100644 --- a/packages/base/card-api.gts +++ b/packages/base/card-api.gts @@ -2397,10 +2397,10 @@ export function containsMany( options?: Options, ): BaseInstanceType[] { return { - setupField(fieldName: string, _ownerPrototype: BaseDef) { + setupField(fieldName: string, ownerPrototype: BaseDef) { let { computeVia, searchable } = options ?? {}; let instance = new ContainsMany({ - cardThunk: cardThunk(field), + cardThunk: cardThunk(field, { fieldName, ownerPrototype }), computeVia, name: fieldName, searchable, @@ -2417,10 +2417,10 @@ export function contains( options?: Options, ): BaseInstanceType { return { - setupField(fieldName: string, _ownerPrototype: BaseDef) { + setupField(fieldName: string, ownerPrototype: BaseDef) { let { computeVia, searchable } = options ?? {}; let instance = new Contains({ - cardThunk: cardThunk(field), + cardThunk: cardThunk(field, { fieldName, ownerPrototype }), computeVia, name: fieldName, searchable, @@ -2439,7 +2439,10 @@ export function linksTo( return { setupField(fieldName: string, ownerPrototype: BaseDef) { let { computeVia, searchable, query } = options ?? {}; - let fieldCardThunk = cardThunk(cardOrThunk); + let fieldCardThunk = cardThunk(cardOrThunk, { + fieldName, + ownerPrototype, + }); if (query) { validateRelationshipQuery(ownerPrototype, fieldName, query); } @@ -2465,7 +2468,10 @@ export function linksToMany( return { setupField(fieldName: string, ownerPrototype: BaseDef) { let { computeVia, searchable, query } = options ?? {}; - let fieldCardThunk = cardThunk(cardOrThunk); + let fieldCardThunk = cardThunk(cardOrThunk, { + fieldName, + ownerPrototype, + }); if (query) { validateRelationshipQuery(ownerPrototype, fieldName, query); } @@ -4783,12 +4789,23 @@ function notifySubscribers( function cardThunk( cardOrThunk: CardT | (() => CardT), + // Where the field lives, so the thrown error can name the exact field + // instead of leaving the author to bisect their schema. The module the bad + // value came from isn't knowable here — by the time the value reaches us + // the import has already evaluated to undefined — so the message names the + // two ways that happens instead. + fieldContext?: { fieldName: string; ownerPrototype: BaseDef }, ): () => CardT { if (!cardOrThunk) { + let fieldDescription = fieldContext + ? `field '${fieldContext.fieldName}' on '${ + fieldContext.ownerPrototype.constructor?.name ?? 'unknown card' + }'` + : 'a field'; throw new Error( - `cardOrThunk was ${cardOrThunk}. There might be a cyclic dependency in one of your fields. - Use '() => CardName' format for the fields with the cycle in all related cards. - e.g.: '@field friend = linksTo(() => Person)'`, + `The card class for ${fieldDescription} was ${cardOrThunk}. Two common causes: + (1) the import doesn't match the module's export shape — e.g. \`import X from '…'\` where the module only has a named export; use \`import { X } from '…'\`; + (2) a cyclic dependency between cards — use the thunk form in all cards in the cycle, e.g. '@field friend = linksTo(() => Person)'.`, ); } return ( diff --git a/packages/base/csv-file-def.gts b/packages/base/csv-file-def.gts index 42316cf3bb1..3fbc4e3e309 100644 --- a/packages/base/csv-file-def.gts +++ b/packages/base/csv-file-def.gts @@ -463,3 +463,5 @@ export class CsvFileDef extends FileDef { }; } } + +export default CsvFileDef; diff --git a/packages/base/flac-audio-def.gts b/packages/base/flac-audio-def.gts index 343c69374ac..a4d38441aeb 100644 --- a/packages/base/flac-audio-def.gts +++ b/packages/base/flac-audio-def.gts @@ -54,3 +54,5 @@ export class FlacDef extends AudioDef { }; } } + +export default FlacDef; diff --git a/packages/base/gif-image-def.gts b/packages/base/gif-image-def.gts index d2dcd3d2a65..2b2edc33763 100644 --- a/packages/base/gif-image-def.gts +++ b/packages/base/gif-image-def.gts @@ -43,3 +43,5 @@ export class GifDef extends RasterImageDef { }; } } + +export default GifDef; diff --git a/packages/base/gts-file-def.gts b/packages/base/gts-file-def.gts index e056b61aff0..fcfb4769eee 100644 --- a/packages/base/gts-file-def.gts +++ b/packages/base/gts-file-def.gts @@ -12,3 +12,5 @@ export class GtsFileDef extends TsFileDef { // GTS adds, so the inherited CodePreview renders a `.gts` file correctly. static fileKind = 'Glimmer TS'; } + +export default GtsFileDef; diff --git a/packages/base/jpg-image-def.gts b/packages/base/jpg-image-def.gts index f905f3cd9f8..e9edd7500d2 100644 --- a/packages/base/jpg-image-def.gts +++ b/packages/base/jpg-image-def.gts @@ -47,3 +47,5 @@ export class JpgDef extends RasterImageDef { }; } } + +export default JpgDef; diff --git a/packages/base/json-file-def.gts b/packages/base/json-file-def.gts index e814bb48a32..392869b11c9 100644 --- a/packages/base/json-file-def.gts +++ b/packages/base/json-file-def.gts @@ -594,3 +594,5 @@ export class JsonFileDef extends FileDef { }; } } + +export default JsonFileDef; diff --git a/packages/base/m4a-audio-def.gts b/packages/base/m4a-audio-def.gts index cd530d2d8ee..0d983b3a530 100644 --- a/packages/base/m4a-audio-def.gts +++ b/packages/base/m4a-audio-def.gts @@ -55,3 +55,5 @@ export class M4aDef extends AudioDef { }; } } + +export default M4aDef; diff --git a/packages/base/markdown-file-def.gts b/packages/base/markdown-file-def.gts index 11b61bfd574..3b2ee786b1c 100644 --- a/packages/base/markdown-file-def.gts +++ b/packages/base/markdown-file-def.gts @@ -558,3 +558,5 @@ export class MarkdownDef extends FileDef { return attributes; } } + +export default MarkdownDef; diff --git a/packages/base/mp3-audio-def.gts b/packages/base/mp3-audio-def.gts index b0445a0aa9e..3c66ef53a0c 100644 --- a/packages/base/mp3-audio-def.gts +++ b/packages/base/mp3-audio-def.gts @@ -102,3 +102,5 @@ export class Mp3Def extends AudioDef { } } } + +export default Mp3Def; diff --git a/packages/base/ogg-audio-def.gts b/packages/base/ogg-audio-def.gts index c0734558578..3d1709e620b 100644 --- a/packages/base/ogg-audio-def.gts +++ b/packages/base/ogg-audio-def.gts @@ -55,3 +55,5 @@ export class OggDef extends AudioDef { }; } } + +export default OggDef; diff --git a/packages/base/png-image-def.gts b/packages/base/png-image-def.gts index 67ca386bb64..6994bfb306d 100644 --- a/packages/base/png-image-def.gts +++ b/packages/base/png-image-def.gts @@ -43,3 +43,5 @@ export class PngDef extends RasterImageDef { }; } } + +export default PngDef; diff --git a/packages/base/svg-image-def.gts b/packages/base/svg-image-def.gts index aa438ac60c1..f8edfa29a38 100644 --- a/packages/base/svg-image-def.gts +++ b/packages/base/svg-image-def.gts @@ -1,7 +1,7 @@ import { byteStreamToUint8Array } from '@cardstack/runtime-common'; import SvgIcon from '@cardstack/boxel-icons/file-type-svg'; import ImageDef from './image-file-def'; -import { type ByteStream, type SerializedFile } from './file-api'; +import type { ByteStream, SerializedFile } from './file-api'; import { extractSvgDimensions } from './svg-meta-extractor'; export class SvgDef extends ImageDef { @@ -25,3 +25,5 @@ export class SvgDef extends ImageDef { }; } } + +export default SvgDef; diff --git a/packages/base/text-file-def.gts b/packages/base/text-file-def.gts index 50ec0f1a362..f43ca680526 100644 --- a/packages/base/text-file-def.gts +++ b/packages/base/text-file-def.gts @@ -243,3 +243,5 @@ export class TextFileDef extends FileDef { }; } } + +export default TextFileDef; diff --git a/packages/base/ts-file-def.gts b/packages/base/ts-file-def.gts index 63a2241c013..0ce19d7857c 100644 --- a/packages/base/ts-file-def.gts +++ b/packages/base/ts-file-def.gts @@ -303,3 +303,5 @@ export class TsFileDef extends FileDef { }; } } + +export default TsFileDef; diff --git a/packages/base/wav-audio-def.gts b/packages/base/wav-audio-def.gts index b65ae406fa5..712df4c660c 100644 --- a/packages/base/wav-audio-def.gts +++ b/packages/base/wav-audio-def.gts @@ -75,3 +75,5 @@ export class WavDef extends AudioDef { }; } } + +export default WavDef; diff --git a/packages/base/webp-image-def.gts b/packages/base/webp-image-def.gts index fc7de82f566..e0f3247b081 100644 --- a/packages/base/webp-image-def.gts +++ b/packages/base/webp-image-def.gts @@ -39,3 +39,5 @@ export class WebpDef extends RasterImageDef { }; } } + +export default WebpDef; diff --git a/packages/host/tests/unit/card-thunk-error-test.ts b/packages/host/tests/unit/card-thunk-error-test.ts new file mode 100644 index 00000000000..5934c535e3d --- /dev/null +++ b/packages/host/tests/unit/card-thunk-error-test.ts @@ -0,0 +1,36 @@ +import { getService } from '@universal-ember/test-support'; +import { module, test } from 'qunit'; + +import { setupRenderingTest } from '../helpers/setup'; + +// When a field's card class evaluates to undefined — most often a default +// import of a module that only has named exports, or a genuine cycle — the +// thrown error names the field and its owning card, so the author lands on +// the exact declaration instead of bisecting the schema. These call the +// `field` decorator the same way Babel's decorator transform does. +module('Unit | card field thunk errors', function (hooks) { + setupRenderingTest(hooks); + + test('an undefined field card class names the field and its owner', async function (assert) { + let loader = getService('loader-service').loader; + let api = (await loader.import('@cardstack/base/card-api')) as any; + class Broken extends api.CardDef {} + + let cases: [string, string, (value: any) => unknown][] = [ + ['linksTo', 'src', api.linksTo], + ['linksToMany', 'links', api.linksToMany], + ['contains', 'meta', api.contains], + ['containsMany', 'items', api.containsMany], + ]; + for (let [label, fieldName, fieldFn] of cases) { + assert.throws( + () => + api.field(Broken.prototype, fieldName, { + initializer: () => fieldFn(undefined), + }), + new RegExp(`field '${fieldName}' on 'Broken'`), + `${label} names the field and owner`, + ); + } + }); +}); diff --git a/packages/host/tests/unit/filedef-export-shape-test.ts b/packages/host/tests/unit/filedef-export-shape-test.ts new file mode 100644 index 00000000000..5d44e716b55 --- /dev/null +++ b/packages/host/tests/unit/filedef-export-shape-test.ts @@ -0,0 +1,53 @@ +import { getService } from '@universal-ember/test-support'; +import { module, test } from 'qunit'; + +import { FILEDEF_CODE_REF_BY_EXTENSION } from '@cardstack/runtime-common'; + +import { setupRenderingTest } from '../helpers/setup'; + +// Every FileDef subtype module exports its def class both named and as the +// default, so `import X from '…'` and `import { X } from '…'` are equally +// valid. A named-only module makes a default import silently evaluate to +// undefined, which then fails far away at schema time; a consistent shape +// removes the trap. Walking the extension registry keeps this guard covering +// every registered subtype, including ones added later. +module('Unit | FileDef subtype export shapes', function (hooks) { + setupRenderingTest(hooks); + + test('every registered subtype module has matching named and default exports', async function (assert) { + let loader = getService('loader-service').loader; + let seen = new Set(); + for (let [extension, ref] of Object.entries( + FILEDEF_CODE_REF_BY_EXTENSION, + )) { + // `.mismatch` is a synthetic registry entry for a non-existent module, + // not a real subtype. + if (extension === '.mismatch') { + continue; + } + let key = `${ref.module}#${ref.name}`; + if (seen.has(key)) { + continue; + } + seen.add(key); + let ns = (await loader.import(ref.module)) as Record; + assert.ok(ns[ref.name], `${ref.module} has named export ${ref.name}`); + assert.ok(ns['default'], `${ref.module} has a default export`); + // A module registered under exactly one class defaults to that class. + // Multi-leaf modules (e.g. gltf-model-def's GltfDef/GlbDef) default to + // one of their leaves; existence is all that's required there. + let namesInModule = new Set( + Object.values(FILEDEF_CODE_REF_BY_EXTENSION) + .filter((r) => r.module === ref.module) + .map((r) => r.name), + ); + if (namesInModule.size === 1) { + assert.strictEqual( + ns['default'], + ns[ref.name], + `${ref.module} default export is ${ref.name}`, + ); + } + } + }); +}); diff --git a/packages/runtime-common/file-def-code-ref.ts b/packages/runtime-common/file-def-code-ref.ts index 929ddf0ac11..c689b3ba209 100644 --- a/packages/runtime-common/file-def-code-ref.ts +++ b/packages/runtime-common/file-def-code-ref.ts @@ -15,7 +15,7 @@ function baseModule(name: string): RealmResourceIdentifier { return `${baseRealm.url}${name}` as RealmResourceIdentifier; } -const FILEDEF_CODE_REF_BY_EXTENSION: Record = { +export const FILEDEF_CODE_REF_BY_EXTENSION: Record = { // TODO: Replace with realm metadata configuration. '.markdown': { module: baseModule('markdown-file-def'), name: 'MarkdownDef' }, '.md': { module: baseModule('markdown-file-def'), name: 'MarkdownDef' }, From fb7a8880c1818177ed1b4ff538ed20c5f3530b17 Mon Sep 17 00:00:00 2001 From: Luke Melia Date: Tue, 18 Aug 2026 11:25:23 -0400 Subject: [PATCH 2/2] Address review: guard the thunk form, widen export-shape coverage - cardThunk now also guards the thunk form: a thunk resolving to undefined throws the same field-and-owner-named error at first field.card read, instead of an anonymous TypeError later in getFieldDefinitions. fieldContext is required (all call sites pass it), and the owner name falls back via || so an anonymous class's empty-string name doesn't slip through. - FILEDEF_CODE_REF_BY_EXTENSION is Readonly; every use is a read. - The export-shape walk skips synthetic entries by shape (bare relative specifier) rather than by literal key, covers the six family base modules a subtype author imports from, and its comments now cite the real multi-class modules (image-file-def, zip-file-def) and state the guarantee precisely: a default import never yields undefined, not "yields the class you named". - card-thunk-error-test asserts the guidance text (export shape + cyclic dependency) alongside the field naming, and adds the thunk-form cases through all four field types. Co-Authored-By: Claude Fable 5 --- packages/base/card-api.gts | 38 ++++++---- .../host/tests/unit/card-thunk-error-test.ts | 62 ++++++++++++---- .../tests/unit/filedef-export-shape-test.ts | 71 +++++++++++++++---- packages/runtime-common/file-def-code-ref.ts | 4 +- 4 files changed, 134 insertions(+), 41 deletions(-) diff --git a/packages/base/card-api.gts b/packages/base/card-api.gts index 831a53ccfe9..b4f970e8457 100644 --- a/packages/base/card-api.gts +++ b/packages/base/card-api.gts @@ -4794,23 +4794,37 @@ function cardThunk( // value came from isn't knowable here — by the time the value reaches us // the import has already evaluated to undefined — so the message names the // two ways that happens instead. - fieldContext?: { fieldName: string; ownerPrototype: BaseDef }, + fieldContext: { fieldName: string; ownerPrototype: BaseDef }, ): () => CardT { + // `||` rather than `??`: an anonymous class's `.name` is `''`, not nullish. + let fieldDescription = `field '${fieldContext.fieldName}' on '${ + fieldContext.ownerPrototype.constructor?.name || 'unknown card' + }'`; + let causes = `Two common causes: + (1) the import doesn't match the module's export shape — e.g. \`import X from '…'\` where the module only has a named export; use \`import { X } from '…'\`; + (2) a cyclic dependency between cards — use the thunk form in all cards in the cycle, e.g. '@field friend = linksTo(() => Person)'.`; if (!cardOrThunk) { - let fieldDescription = fieldContext - ? `field '${fieldContext.fieldName}' on '${ - fieldContext.ownerPrototype.constructor?.name ?? 'unknown card' - }'` - : 'a field'; throw new Error( - `The card class for ${fieldDescription} was ${cardOrThunk}. Two common causes: - (1) the import doesn't match the module's export shape — e.g. \`import X from '…'\` where the module only has a named export; use \`import { X } from '…'\`; - (2) a cyclic dependency between cards — use the thunk form in all cards in the cycle, e.g. '@field friend = linksTo(() => Person)'.`, + `The card class for ${fieldDescription} was ${cardOrThunk}. ${causes}`, ); } - return ( - 'baseDef' in cardOrThunk ? () => cardOrThunk : cardOrThunk - ) as () => CardT; + if ('baseDef' in cardOrThunk) { + return () => cardOrThunk as CardT; + } + // The thunk form fails the same two ways the eager form does, just later — + // at the first `field.card` read instead of at decoration. The accessors + // re-invoke the thunk on every read (nothing is memoized), so this adds a + // truthiness check per read, not an extra call. + let thunk = cardOrThunk as () => CardT; + return () => { + let card = thunk(); + if (!card) { + throw new Error( + `The card class thunk for ${fieldDescription} returned ${card}. ${causes}`, + ); + } + return card; + }; } export type SignatureFor = { diff --git a/packages/host/tests/unit/card-thunk-error-test.ts b/packages/host/tests/unit/card-thunk-error-test.ts index 5934c535e3d..d2bddcf8d43 100644 --- a/packages/host/tests/unit/card-thunk-error-test.ts +++ b/packages/host/tests/unit/card-thunk-error-test.ts @@ -5,31 +5,67 @@ import { setupRenderingTest } from '../helpers/setup'; // When a field's card class evaluates to undefined — most often a default // import of a module that only has named exports, or a genuine cycle — the -// thrown error names the field and its owning card, so the author lands on -// the exact declaration instead of bisecting the schema. These call the -// `field` decorator the same way Babel's decorator transform does. +// thrown error names the field and its owning card and spells out both +// causes, so the author lands on the exact declaration instead of bisecting +// the schema. The eager form throws at decoration time; the thunk form +// throws at the first `field.card` read. These call the `field` decorator +// the same way Babel's decorator transform does. module('Unit | card field thunk errors', function (hooks) { setupRenderingTest(hooks); - test('an undefined field card class names the field and its owner', async function (assert) { + let fieldKinds = (api: any): [string, string, (value: any) => unknown][] => [ + ['linksTo', 'src', api.linksTo], + ['linksToMany', 'links', api.linksToMany], + ['contains', 'meta', api.contains], + ['containsMany', 'items', api.containsMany], + ]; + + test('an undefined field card class names the field, the owner, and both causes', async function (assert) { let loader = getService('loader-service').loader; let api = (await loader.import('@cardstack/base/card-api')) as any; class Broken extends api.CardDef {} - let cases: [string, string, (value: any) => unknown][] = [ - ['linksTo', 'src', api.linksTo], - ['linksToMany', 'links', api.linksToMany], - ['contains', 'meta', api.contains], - ['containsMany', 'items', api.containsMany], - ]; - for (let [label, fieldName, fieldFn] of cases) { + for (let [label, fieldName, fieldFn] of fieldKinds(api)) { assert.throws( () => api.field(Broken.prototype, fieldName, { initializer: () => fieldFn(undefined), }), - new RegExp(`field '${fieldName}' on 'Broken'`), - `${label} names the field and owner`, + (err: Error) => + new RegExp(`field '${fieldName}' on 'Broken'`).test(err.message) && + /export shape/.test(err.message) && + /cyclic dependency/.test(err.message), + `${label} names the field, the owner, and both causes`, + ); + } + }); + + test('a thunk resolving to undefined names the field at first read', async function (assert) { + let loader = getService('loader-service').loader; + let api = (await loader.import('@cardstack/base/card-api')) as any; + let { isField } = (await loader.import('@cardstack/runtime-common')) as any; + class BrokenThunk extends api.CardDef {} + + for (let [label, fieldName, fieldFn] of fieldKinds(api)) { + // The thunk defers evaluation, so decoration itself succeeds. The + // decorator returns the property descriptor whose getter carries the + // Field object; `field.card` is the accessor every consumer reads + // through. + let descriptor = api.field(BrokenThunk.prototype, fieldName, { + initializer: () => fieldFn(() => undefined), + }); + let field = (descriptor?.get as any)?.[isField]; + assert.ok(field, `${label} field is registered`); + // …and the named error surfaces the first time the class is needed. + assert.throws( + () => field.card, + (err: Error) => + new RegExp(`field '${fieldName}' on 'BrokenThunk'`).test( + err.message, + ) && + /export shape/.test(err.message) && + /cyclic dependency/.test(err.message), + `${label} thunk form names the field and owner at first read`, ); } }); diff --git a/packages/host/tests/unit/filedef-export-shape-test.ts b/packages/host/tests/unit/filedef-export-shape-test.ts index 5d44e716b55..ff8b395d022 100644 --- a/packages/host/tests/unit/filedef-export-shape-test.ts +++ b/packages/host/tests/unit/filedef-export-shape-test.ts @@ -1,28 +1,39 @@ import { getService } from '@universal-ember/test-support'; import { module, test } from 'qunit'; -import { FILEDEF_CODE_REF_BY_EXTENSION } from '@cardstack/runtime-common'; +import { + FILEDEF_CODE_REF_BY_EXTENSION, + baseRealm, +} from '@cardstack/runtime-common'; import { setupRenderingTest } from '../helpers/setup'; -// Every FileDef subtype module exports its def class both named and as the -// default, so `import X from '…'` and `import { X } from '…'` are equally -// valid. A named-only module makes a default import silently evaluate to -// undefined, which then fails far away at schema time; a consistent shape -// removes the trap. Walking the extension registry keeps this guard covering -// every registered subtype, including ones added later. +// Every FileDef module exports its def class both named and as the default, +// so `import X from '…'` and `import { X } from '…'` are equally valid. A +// named-only module makes a default import silently evaluate to undefined, +// which then fails far away at schema time; a consistent shape removes the +// trap. Walking the extension registry keeps this guard covering every +// registered subtype, including ones added later. +// +// What this pins is "a default import never yields undefined" — not "a +// default import yields the class you named". A module carrying a second, +// named-only class is the sharper trap: `import RasterImageDef from +// './image-file-def'` evaluates to ImageDef (the default), a perfectly valid +// class, so nothing throws and the field is silently wired to the wrong +// class. image-file-def (RasterImageDef) and zip-file-def (ArchiveEntryField) +// are the two such modules today; only their registered class is pinned to +// the default below. module('Unit | FileDef subtype export shapes', function (hooks) { setupRenderingTest(hooks); test('every registered subtype module has matching named and default exports', async function (assert) { let loader = getService('loader-service').loader; let seen = new Set(); - for (let [extension, ref] of Object.entries( - FILEDEF_CODE_REF_BY_EXTENSION, - )) { - // `.mismatch` is a synthetic registry entry for a non-existent module, - // not a real subtype. - if (extension === '.mismatch') { + for (let ref of Object.values(FILEDEF_CODE_REF_BY_EXTENSION)) { + // Real entries are built by `baseModule()` in full-URL form; a bare + // relative specifier marks a synthetic test-only entry with no module + // behind it (`.mismatch` today). + if (!ref.module.includes('://')) { continue; } let key = `${ref.module}#${ref.name}`; @@ -34,8 +45,10 @@ module('Unit | FileDef subtype export shapes', function (hooks) { assert.ok(ns[ref.name], `${ref.module} has named export ${ref.name}`); assert.ok(ns['default'], `${ref.module} has a default export`); // A module registered under exactly one class defaults to that class. - // Multi-leaf modules (e.g. gltf-model-def's GltfDef/GlbDef) default to - // one of their leaves; existence is all that's required there. + // A module that ever registers two leaf classes would default to one + // of them; there, existence of *a* default is all this walk can + // require (see the module comment for why that case is a trap of its + // own). let namesInModule = new Set( Object.values(FILEDEF_CODE_REF_BY_EXTENSION) .filter((r) => r.module === ref.module) @@ -50,4 +63,32 @@ module('Unit | FileDef subtype export shapes', function (hooks) { } } }); + + // The registry maps extensions to leaf subtypes only, so the family's base + // modules — the ones a subtype author actually imports from — appear in no + // entry and the walk above never touches them. One of these defaults is + // already load-bearing in the shipped tree: svg-image-def default-imports + // image-file-def. + test('family base modules have matching named and default exports', async function (assert) { + let loader = getService('loader-service').loader; + let baseModules: { module: string; name: string }[] = [ + { module: 'image-file-def', name: 'ImageDef' }, + { module: 'audio-file-def', name: 'AudioDef' }, + { module: 'video-file-def', name: 'VideoDef' }, + { module: 'font-file-def', name: 'FontDef' }, + { module: 'three-d-model-def', name: 'ThreeDModelDef' }, + { module: 'file-api', name: 'FileDef' }, + ]; + for (let { module: moduleName, name } of baseModules) { + let moduleId = `${baseRealm.url}${moduleName}`; + let ns = (await loader.import(moduleId)) as Record; + assert.ok(ns[name], `${moduleId} has named export ${name}`); + assert.ok(ns['default'], `${moduleId} has a default export`); + assert.strictEqual( + ns['default'], + ns[name], + `${moduleId} default export is ${name}`, + ); + } + }); }); diff --git a/packages/runtime-common/file-def-code-ref.ts b/packages/runtime-common/file-def-code-ref.ts index c689b3ba209..80918ab58fd 100644 --- a/packages/runtime-common/file-def-code-ref.ts +++ b/packages/runtime-common/file-def-code-ref.ts @@ -15,7 +15,9 @@ function baseModule(name: string): RealmResourceIdentifier { return `${baseRealm.url}${name}` as RealmResourceIdentifier; } -export const FILEDEF_CODE_REF_BY_EXTENSION: Record = { +export const FILEDEF_CODE_REF_BY_EXTENSION: Readonly< + Record +> = { // TODO: Replace with realm metadata configuration. '.markdown': { module: baseModule('markdown-file-def'), name: 'MarkdownDef' }, '.md': { module: baseModule('markdown-file-def'), name: 'MarkdownDef' },