fix: register data models under their camelCase record type - #464
Open
herzzanu wants to merge 1 commit into
Open
fix: register data models under their camelCase record type#464herzzanu wants to merge 1 commit into
herzzanu 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.
Author
|
@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: |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.