Breaking change ED 2.11.1 to 2.11.2

[lindyhopchris]

Really useful package!

So I've got an app that was working fine with Model Fragments 2.11.1 and Ember Data 2.11.1. However when I upgrade ED to 2.11.2 or 2.11.3 I get an error.

The error I'm hitting is this assertion:
https://github.com/lytics/ember-data-model-fragments/blob/master/addon/transforms/fragment.js#L59

With ED 2.11.1 the serializer for the fragment must be a JSONSerializer whereas with the upgraded ED it's a JSONAPISerializer.

My setup is that I've got a Fragment serializer:

// app/pods/fragment/serializer.js
import Ember from 'ember';
import DS from 'ember-data';

const { String: { dasherize } } = Ember;
const { JSONSerializer } = DS;

export default JSONSerializer.extend({
  keyForAttribute(attr) {
    return dasherize(attr);
  }
});

And an initializer as per your README:

// app/initializers/fragment-serializer.js
import FragmentSerializer from '../pods/fragment/serializer';

export function initialize(application) {
    application.register('serializer:-fragment', FragmentSerializer);
}

export default {
    name: 'fragment-serializer',
    initialize: initialize
};

Am I doing something wrong that shouldn't have worked in the first place? Or has the ED change introduced something breaking?

[workmanw]

Hmm. I can't see anything obvious that is wrong with your code. Also looking at the ember-data changelog I don't see anything that sticks out. I'll see if I can make some time today or tomorrow to investigate.

[workmanw]

Yeap, it seems our tests are broken.

re`: the store has an `isFragment` method
not ok 186 PhantomJS 2.1 - unit - `DS.Store`: the default fragment serializer does not use the application serializer
    ---
        actual: >
            false
        expected: >
            true
        stack: >
            http://localhost:7357/assets/tests.js:4337:14
            runTest@http://localhost:7357/assets/test-support.js:3257:32
            run@http://localhost:7357/assets/test-support.js:3242:11
            http://localhost:7357/assets/test-support.js:3384:14
            process@http://localhost:7357/assets/test-support.js:3043:24
            begin@http://localhost:7357/assets/test-support.js:3025:9
            http://localhost:7357/assets/test-support.js:3085:9
        message: >
            fragment serializer fallback is not `JSONAPISerializer`
        Log: |
    ...
ok 187 PhantomJS 2.1 - unit - `DS.Store`: the default fragment serializer does not use the adapter's `defaultSerializer`
not ok 188 PhantomJS 2.1 - unit - `DS.Store`: the default fragment serializer is `serializer:-fragment` if registered
    ---
        actual: >
            false
        expected: >
            true
        stack: >
            http://localhost:7357/assets/tests.js:4352:14
            runTest@http://localhost:7357/assets/test-support.js:3257:32
            run@http://localhost:7357/assets/test-support.js:3242:11
            http://localhost:7357/assets/test-support.js:3384:14
            process@http://localhost:7357/assets/test-support.js:3043:24
            begin@http://localhost:7357/assets/test-support.js:3025:9
            http://localhost:7357/assets/test-support.js:3085:9
        message: >
            fragment serializer fallback is correct
        Log: |
    ...

[lindyhopchris]

Yep, that looks like the bug. Thanks for looking into it!

[workmanw]

@lindyhopchris It turns out that I'm inadvertently the culprit here. I filled: emberjs/data#4810 to fix another injection issue. And it was fixed by: emberjs/data#4810 (and backported to release). Unfortuantely, that breaks our override logic here: addon/ext.js#L371.

The factoryFor stuff has turned out to be quite hairy. Oh yea, and I was one of the people who had originally requested that feature emberjs/rfcs#125 . 😬 😬 😬 😬 😬

I'll get this fixed today or tomorrow. I need to think about the right solution here to not break backwards compatibility. Thanks for reporting this!

[lindyhopchris]

No problem! I've tied my app to 2.11.1 so this isn't causing me major problems at the moment, but glad to hear you're on the case.

[workmanw]

@lindyhopchris Would you be able to confirm that this is fixed on master? I'd appreciate it.

[jakesjews]

@workmanw I just tried upgrading to ember-data 2.11.3 and am running into this bug too so i can check if this fixes it.

[jakesjews]

@workmanw I can confirm this issue is fixed on master

[workmanw]

@jakesjews Excellent. Thank you! Obviously we need to have another point release. There are two quick things I want to look into first. ember-cli/ember-twiddle#531 and I saw a deprecation warnings when I was running the test locally. I want to be sure it would create deprecation warnings for users. Should be ready to go by tomorrow.

[lindyhopchris]

@workmanw this is also fixed for me using master. Thanks!