fix: register data models under their camelCase record type - #464
fix: register data models under their camelCase record type#464herzzanu wants to merge 1 commit into
Conversation
Model files are kebab-case on disk, so setupOrbit received keys like '../data-models/planetary-system.ts' and registered the model as 'planetary-system'. Both consumers of registrations.models look models up by camelCase record type instead: - DataSchema builds its model map from getRegisteredModels(), which camelized the keys before indexing the un-camelized registry, so the lookup returned undefined and destructuring threw during setupOrbit. - Cache#modelFactoryFor(type) is called with the record type, so it hit the same mismatch. Camelize once at registration and let getRegisteredModels return the keys as stored, so registration and lookup agree. Single-word names were unaffected, which is why the existing suite passed: tests supply already-camelCase keys to createStore, and the test app's data-models folder only holds moon.ts and planet.ts.
|
@RobbieTheWagner @NullVoxPopuli — small fix with a red/green test, would appreciate a look when you have a moment. |
|
@herzzanu with this I get a build error: |
|
@lucacorti that one's not coming from this PR — it's how the branch is being installed.
It doesn't self-build on install because Easiest way to try the branch is to build a tarball: git clone -b fix-kebab-case-model-names https://github.com/herzzanu/ember-orbit
cd ember-orbit && pnpm install && pnpm build # emits dist/ + declarations/
npm pack # -> ember-orbit-0.20.0.tgz
# then in your app:
pnpm add file:../ember-orbit/ember-orbit-0.20.0.tgzSame applies if you linked a checkout — Alternatively, patch the released
Would be good to hear whether the fix actually resolves your #431 schema errors once it's building. |
Fixes #463.
Model files are kebab-case on disk, so
setupOrbitreceives glob keys like../data-models/planetary-system.tsand registers the model underplanetary-system. Both consumers ofregistrations.modelslook models up by camelCase record type instead, so every multi-word model is unreachable:DataSchemabuilds its model map fromgetRegisteredModels(), which camelized the keys and then indexed the un-camelized registry — the lookup returnsundefinedand destructuring throws duringsetupOrbit.Cache#modelFactoryFor(type)is called with the record type, so it hits the same mismatch and asserts "An ember-orbit model for type … has not been registered."The change
Camelize once at registration, and let
getRegisteredModels()return the keys as stored, so registration and lookup agree. Registry keys now match the record types used everywhere else, which is also whatmodelFactoryForalready assumed.The alternative — keeping the registry kebab-cased and camelizing only the schema's model names — would leave
modelFactoryForneeding its own conversion, so normalizing at the single write point seemed cleaner. Happy to switch if you'd prefer it the other way.Why the existing suite didn't catch this
Both paths that register models avoid real kebab-case file names:
tests/support/store.ts#createStore, which synthesizes module keys from the dict keys the test passes — and those are already camelCase (binaryStar,planetarySystem).tests/test-app/data-models/only containsmoon.tsandplanet.ts, andcamelize('moon') === 'moon'.The added test closes that gap by registering a fixture under a kebab-case key, the way a real glob would.
Verification
Run against this repo's suite (Chrome; Firefox isn't installed locally):
TypeError: Cannot destructure property 'keys' of 'orbitRegistry.registrations.models[name]' as it is undefinedprettier --check,eslintandtsc --noEmitare clean.I also hit this on a classic ember-cli app (via
ember-classic-import-meta-glob), where it prevented boot entirely; the failing test here runs through the Vite build, so it isn't builder-specific.