Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Not working with ember-data v3.2.0-beta.2 #296

Open
dwickern opened this issue May 1, 2018 · 5 comments
Open

Not working with ember-data v3.2.0-beta.2 #296

dwickern opened this issue May 1, 2018 · 5 comments

Comments

@dwickern
Copy link
Contributor

dwickern commented May 1, 2018

After upgrading from 3.2.0-beta.1 to 3.2.0-beta.2 I've got some issues related to serializing fragments:

Error: Assertion Failed: You need to pass a model name to the store's serializerFor method
    at new EmberError (assets/vendor.js:36279:25)
    at Object.assert (assets/vendor.js:36522:15)
    at Class.serializerFor (assets/vendor.js:395562:49)
    at Class.superWrapper [as serializerFor] (assets/vendor.js:67773:22)
    at Class.serializeFragment [as serialize] (assets/vendor.js:396431:30)
    at Class.serializeAttribute (assets/vendor.js:451412:29)
    at snapshot.eachAttribute (assets/vendor.js:451342:14)
    at Ember.get.forEach (assets/vendor.js:436820:16)
    at Map.forEach (<anonymous>)
    at MapWithDeprecations.forEach (assets/vendor.js:433902:22)

This project's tests don't pass either. git bisect points to emberjs/data#5413

/cc @runspired

@runspired
Copy link
Contributor

Given the commit the bisect points at, I suspect model-fragments has a similar race condition to the one we solved in it.

@runspired
Copy link
Contributor

The root cause is that internalModel is now responsible for deciding the current UI value instead of the attr on the record. This allows us to be lazier about materializing the record. Fragments is currently dependent on the record being materialized and the properties accessed in order to setup _fragments. This was always likely to be buggy, but now it is even more so. However, we can patch internalModel.setupData similar to our other method extensions to properly setup fragments. I'll have a PR up shortly.

@jakesjews
Copy link
Contributor

@runspired thanks a ton! I spent some time trying to pinpoint what the issue was and this helps alot.

@rondale-sc
Copy link
Contributor

Not sure where to put this, but I have some code that in ED 3.1 worked:

this.store.createRecord('something', { someModelFragement: { foo: 1 } });

That now requires I set

let something = this.store.createRecord('something');
something.set('someModelFragement', store.createFragment('fragement-name', {foo: 1});

Was the former never supposed to work?

@rondale-sc
Copy link
Contributor

I did some spelunking today to see if I could write something that'd follow along with @runspired's comment above, but found it to be a bit more complex than I had originally thought.

@runspired Would you be able to spend a little bit of time explaining the thought process of your fix and I'll implement? I got so far as creating a simple dummy repo with my above example and linking edmf. It doesn't seem as if in createRecord flow that internalModel.setupData is ever called.

Instead it calls:

// in store
let internalModel = this._buildInternalModel
internalModel.loadedData(); // which just emits a data loaded event
return internalModel.getRecord(properties); // which has logic to construct a record given `attributes` `belongsTo` and `hasMany`.

I'm stuck on what function we should extend.

Any help would be very much appreciated. 🍻

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants