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..b4f970e8457 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,17 +4789,42 @@ 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 { + // `||` 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) { 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}. ${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/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..d2bddcf8d43 --- /dev/null +++ b/packages/host/tests/unit/card-thunk-error-test.ts @@ -0,0 +1,72 @@ +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 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); + + 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 {} + + for (let [label, fieldName, fieldFn] of fieldKinds(api)) { + assert.throws( + () => + api.field(Broken.prototype, fieldName, { + initializer: () => fieldFn(undefined), + }), + (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 new file mode 100644 index 00000000000..ff8b395d022 --- /dev/null +++ b/packages/host/tests/unit/filedef-export-shape-test.ts @@ -0,0 +1,94 @@ +import { getService } from '@universal-ember/test-support'; +import { module, test } from 'qunit'; + +import { + FILEDEF_CODE_REF_BY_EXTENSION, + baseRealm, +} from '@cardstack/runtime-common'; + +import { setupRenderingTest } from '../helpers/setup'; + +// 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 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}`; + 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. + // 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) + .map((r) => r.name), + ); + if (namesInModule.size === 1) { + assert.strictEqual( + ns['default'], + ns[ref.name], + `${ref.module} default export is ${ref.name}`, + ); + } + } + }); + + // 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 929ddf0ac11..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; } -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' },