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

Error proxy set handler returned false for property '"_super"' in 4.7.3 #8202

Open
jrjohnson opened this issue Oct 3, 2022 · 5 comments
Open
Labels
upstream Upstream Dependency Fix Required

Comments

@jrjohnson
Copy link
Sponsor Contributor

Reproduction

I'm not sure this is testing the same thing, but it fails in the same way and its the closest I can get to a reproduction, I wasn't able to make this fail in the same way in a brand new app:

//packages/-ember-data/tests/unit/record-arrays/record-array-test.js

deprecatedTest(
    'wrap record array in an ember array',
    { id: 'ember-data:deprecate-array-like', until: '5.0', count: 3 },
    async function (assert) {
      this.owner.register('model:tag', Tag);
      let store = this.owner.lookup('service:store');

      let records = store.push({
        data: [
          {
            type: 'tag',
            id: '1',
          },
        ],
      });

      let recordArray = new RecordArray({
        type: 'recordType',
        identifiers: records.map(recordIdentifierFor),
        store,
      });

      let proxiedRecordArray = emberArray(recordArray);

      assert.strictEqual(proxiedRecordArray.length, 1);
    }
  );

Description

I'm seeing an error proxy set handler returned false for property '"_super"' in ED 4.7.3 which seems to be caused by wrapping the result of store.query('foo') in a Ember.A which happens, for us, in the includes helper from ember-composable-helpers

Versions

npm list ember-source && npm list ember-cli && npm list --pattern ember-data
ilios-common@71.0.1
├─┬ @ember/render-modifiers@2.0.4
│ └── ember-source@4.7.0 deduped
├─┬ @ember/test-helpers@2.8.1
│ └── ember-source@4.7.0 deduped
├─┬ @embroider/util@1.8.3
│ └── ember-source@4.7.0 deduped
└── ember-source@4.7.0

ilios-common@71.0.1
├─┬ ember-cli-dependency-checker@3.3.1
│ └── ember-cli@4.3.0 deduped
└── ember-cli@4.3.0

ilios-common@71.0.1
├─┬ ember-cli-mirage@3.0.0-alpha.3
│ └── ember-data@4.7.3 deduped
└── ember-data@4.7.3
@runspired
Copy link
Contributor

IIRC you turned off Prototype extensions. With prototype extensions turned off A changes behavior from a no-op in most cases to a clobber the instance approach (see also my discussion of this in emberjs/rfcs#848 (comment)

This error suggests that length has still been mutated and expects to make a super call.

Incidentally this mutation is why EmberData added a deprecation for calling A(recordArray), and attempts to trick Ember into believing that A has already done it's work.

EmberData's own tests should also have prototype extensions disabled, so it's somewhat confusing that we don't observe this error in our deprecation test here (PromiseManyArray and RecordArrays take the same approach to handling this problem):

'PromiseManyArray is not side-affected by EmberArray',

@jrjohnson
Copy link
Sponsor Contributor Author

Thanks, @runspired I was able to dig into this with your info and discovered that detection logic ED uses which used to happen here seems to have been removed accidentally in emberjs/ember.js#20092 where detecting if something was already an Ember.A was moved and now lives in @ember/-internals/utils/lib/ember-array.ts and appears to require registering each object.

@wagenet
Copy link
Member

wagenet commented Nov 7, 2022

The really strange part is that NativeArray doesn't include length. This leads me to believe something a bit more complex might be going on.

@jrjohnson
Copy link
Sponsor Contributor Author

Turning Array prototype extensions back on has temporarily solved this for us so I'm confident that it is the clobbering of the record by Ember.A() that causes the issue. I used length as an example, but IIRC checking any property on the record results in the error. As the RFC to move away from array prototypes has landed along with a lint fixer I would expect more people are going to start hitting this very soon as they also switch off prototype extensions for array.

@runspired
Copy link
Contributor

the upstream issue in Ember I suspect breaks tons of things regardless of prototype extension status https://discord.com/channels/480462759797063690/486549196837486592/1044738168827674714

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
upstream Upstream Dependency Fix Required
Projects
Status: No status
Development

No branches or pull requests

3 participants