miragejs / ember-cli-mirage

An Ember Addon to easily add Mirage JS to your Ember app.
http://ember-cli-mirage.com
MIT License
863 stars 439 forks source link

`discoverEmberDataModels` crashes if there is a non ember-data object in the app/models folder #2556

Closed andreyfel closed 8 months ago

andreyfel commented 1 year ago

We have a bunch of objects which are plain objects in the app/models folder. We use ember-data@3.28.13. A PR to update ember-cli-mirage from 3.0.0-alpha.5 to 3.0.0 failed. After adding config.store as an argument to discoverEmberDataModels I've got to the following exception:

TypeError: Cannot convert undefined or null to object
    at Function.keys (<anonymous>)
    at ShimModelClass.eachRelationship (http://localhost:4204/assets/vendor.js:111439:14)
    at http://localhost:4204/assets/vendor.js:122501:13
    at Array.forEach (<anonymous>)
    at discoverEmberDataModels (http://localhost:4204/assets/vendor.js:122498:34)
    ...

I'm sure we are not the only ones who have non-ember-data objects in models. Is it possible to just skip them in discoverEmberDataModels rather than be obliged to move them to another folder?

cah-brian-gantzler commented 1 year ago

The code here was always excluding those models. https://github.com/miragejs/ember-cli-mirage/blob/master/packages/ember-cli-mirage/addon/utils/ember-data.js#L14

Now that they are using the store to lookup model, I am assuming this test is no longer being made, or is being made after the store.modelFor call (which is the code returning the shimModelClass https://api.emberjs.com/ember-data/4.1.0/classes/Store/methods/peekAll?anchor=modelFor)

Yea just confirmed it, @SergeAstapov the check if isDsModel should be made BEFORE for the store.modelFor here https://github.com/miragejs/ember-cli-mirage/blob/master/packages/ember-cli-mirage/addon/ember-data.js#L51 as the store.modelFor seems to always return a model (if one doesnt exist, it must be returning this shimModelClass)

SergeAstapov commented 1 year ago

@cah-brian-gantzler unfortunately, we can't do isDsModel before store.modelFor as isDsModel requires model class as an argument as we need to use store.modelFor in latest Ember Data to get ref to model class 🙃

however, I think we can filter out instances of shimModelClass. let me try to play around this and see if it's doable that way.

cah-brian-gantzler commented 1 year ago

Why is it isDsModel required a model class? The whole point of that method is to see if the class you pass it IS a model class?

cloutierlp commented 12 months ago

Anybody found a way to overcome the issue? Same case for us, discoverEmberDataModels fails to discover all models.

corrspt commented 11 months ago

Can't really help @cloutierlp .But I just moved those files to another folder and replaced imports accordingly

azhiv commented 10 months ago

I get the same error against this file <app root>/editors/model.ts which is matched by podModelMatchRegex here resolving to ShimModelClass.