Ember.js: Ember.create(a, undefined, b) is failing after migration to 3.5 from 3.0

[janmisek]

Reason is that CoreObject.create checks whether there is more arguments because of migration to native classes and then it decides how to instantiate. But this check is invalid regarding behaviour of previous ember versions:

https://github.com/emberjs/ember.js/blob/db6a5deeeb0564372159be92d901c804c92266c0/packages/ember-runtime/lib/system/core_object.js#L688

  static create(props, extra) {
    let C = this;

    if (extra === undefined) {
      return new C(props);
    } else {
      return new C(flattenProps.apply(this, arguments));
    }

extra could be undefined, but argument[2] could be defined. In 3.0 and previous versions it was legal to invoke create with second parameter as undefined

This results in incomplete instances.

[rwjblue]

Can you share a snippet that demonstrates the failure? I assume it’s something like:

EmberObject.create({ foo: true }, undefined, { bar: true });

If that’s correct, I’m not sure that we intentionally supported that previously 🤔

[rwjblue]

The new code that you linked is trying to avoid checking arguments.length in this incredibly hot path...

[pzuraq]

This should have thrown an error since v1.0.0: https://github.com/emberjs/ember.js/blame/01523c1496b061e241908905386013722232a4df/packages/ember-runtime/lib/system/core_object.js#L82

I don't think it's a good idea to continue supporting this, but I am curious as to how it was working before 3.x. The assertion has been there this whole time from what I can see.

[pzuraq]

We could possibly add an assertion that checks if arguments.length >= 2 and the second argument is undefined and throws, seems like it would be not ideal though even in dev mode, like @rwjblue mentioned this is a very hot path.

[rwjblue]

Just to be clear: having more than 2 args is fine but they all need to be objects.

A dev mode assertion would probably be fine for this. It would at least give a proper error instead of creating malformed objects...

[janmisek]

EmberObject.create({ foo: true }, undefined, { bar: true });

Yes this is how it could be reproduced.

[janmisek]

My use case from code prior to 3.5 is:

MyClass.create(ownerInjection, options, attrs);

ownerInection is obvious
options are some user options of wrapping method - could be undefined
attrs are my attributes, which I do not want to be overriden by options

But anyway. Its easy to refactor this to some other construct and the code is not anymore in my codebase. But as it was working prior to 3.5 I would consider this as breaking change.

[pzuraq]

Given there was an assertion that was meant to prevent users from passing any arguments other than objects, it was more likely a bugfix. It is unfortunate that this may break user code, but it also was not the intended public API.

[pzuraq]

Closing this issue since the API is operating as intended.