Native Class Constructor Update

[pzuraq]

rendered

[rwjblue]

Great write up, thanks for your continued work on this!

I’m definitey in favor...

[chriskrycho]

😍 with a couple tweaks suggested.

[chriskrycho]

It's worth noting that we've done an unofficial survey of some of the largest apps known to be using ES6 classes in production via TypeScript and have found something like 2 instances of the constructor using injected services total across them. I don't think we have enough data to say that in the RFC, but I wanted to bring it up here for the record in the RFC discussion.

[chriskrycho]

It's worth note that the design landed on here does not break the behavior of any of the fallback solutions the community has had to embrace to deal with the pain point.

As such, this is a case where it's a "breaking change" in the strictest sense but is extremely unlikely to break any real-world code, because the real-world code has necessitated the use of one of those workarounds.

[chriskrycho]

This last sentence is a bit unclear; I actually have no idea what it means. Could you reword?

[pzuraq]

Coming back to it, I think it was pretty unclear too so I'm going to drop it. Basically, it's not possible to easy codemod a class which was base on EmberObject to one which is not based on EmberObject, for a number of reasons:

• Common usage of static methods (create is used everywhere, and tracking down and converting each usage would be very difficult without types in non TS apps)
• Mixins. If they're used once in the inheritance tree, we can't convert.

The point is, we shouldn't be recommending that people use EmberObject at all now that we have native classes, and we should guide users toward converting any existing classes.

[chriskrycho]

I'd suggest "could" rather than "would".

[rwjblue]

I'd like to see this fleshed out to include the deprecation timeline, what would the until be here? I'd personally like to see this be considered a private API deprecation, where until would be the next LTS+1...

[chriskrycho]

I'd also support that timeline.

[pzuraq]

I think the sooner we can remove it, the better. Will update the RFC to state that it will be until: '3.5.0'

[jamesarosen]

I think you mean

Object.assign(instance, props);
instance.init()

[pzuraq]

You are correct, updating!

[rtablada]

I'm excited for native classes and better parity is 👏.

Deprecating new Object feels like a step backwards though. Requiring create seems off in the broader sense of reducing magic and making Ember more comfortable for other JS users.

[pzuraq]

Yeah, I hear you there. Not being able to use new has been problematic in teaching the object model, and is definitely non-standard and not what we want in the future. I wanted to make sure that this decision was made in the larger context of the future of EmberObject in general, which is why I also put forward #338 at the same time as this RFC.

The only time users currently actually manually use create are either when A. they have extended EmberObject directly for their own classes or B. they are writing unit tests. Generally, all other Ember constructs that extend from EmberObject are managed by the container, which means users should never have to manually call create or new for them.

As proposed in #338, the long term goal would be to remove EmberObject from the framework altogether. Classes that users have defined that directly extend EmberObject should actually become native classes, with no base class, and no constructor behavior. However, we cannot codemod current classes that extend EmberObject to these, because of the static class methods like create. We would have to find every instance of the class being created and update it to do something like:

let foo = new Foo();
Object.assign(foo, props);

This could work in some cases, but I don't think it could for all of them safely. The idea with this change is that we do this conversion in two parts:

1. Make converting to native class syntax with EmberObject codemoddable, and easy. Get the community on board, and get everything updated as quickly as possible.
2. Manually update classes that extend EmberObject to no longer need to extend it. This may mean adding a constructor to those classes, it may not. Every class will likely have its own needs and ideal API.

In either case, I believe we should start updating guides, blog posts, addon documentation, etc. to use native classes instead of extending from EmberObject, and generally start to treat it as deprecated and embrace native class syntax.

[pzuraq]

Also, another alternative that has come up is potentially not deprecating new EmberObject(), but deprecating new EmberObject(props). I think this would be a decent middle ground, although it could lead to some confusing branching behavior with init.

[rtablada]

I think deprecating with props is better than deprecating new altogether as public (especially since there was an RFC just in the last year to make new more acceptable).

I'm ok with something like:

let report = new MyReport();
report.setProperties(props);

Where create is just a static shorthand for ☝️?

[rtablada]

From a learning side of things I think we'd need to document the differences in

• constructor
• init
• create

[pzuraq]

I would be OK with it, as long as we made it very, very clear that you should not be extending from EmberObject in general. Overall momentum should be towards writing pure native classes and converting existing classes, and being able to do

let report = new MyReport();
setProperties(report, props); // minor tweak

Is essentially a forwards compatible way to do that.

[Gaurav0]

An alternative not mentioned here is to make the change behind a feature flag enabled by an add-on. This would make the new behavior opt in. We are doing this already for making jQuery optional, for example.

[pzuraq]

That would be a reasonable alternative, we should definitely add it to the alternatives section.

In practice, I think it would be best to ship the target behavior as the actual/only public API and polyfill its behavior to older versions of Ember (already looking into how we can do that, it should be possible). Unlike a lot of the other optional feature flags, we don't technically have a public API that we need to continue to support.

This does beg the question, when does an RFC spec become public API? The process in intentionally fuzzy here, because details may change in implementation (see Named Blocks).

In this case, we've talked to lots of early adopters and they have so far been very supportive of this late-stage update to the original RFC. However, if there are any users of native class syntax out there who would be very against it, we definitely want to hear from them!

[oligriffiths]

I'd like to second the points raised that advising against using new seems like a backwards step.
It seems like allowing new WITH props is the thing that we don't want to support, due to needing these backwards compatible hacks.
It may be worth moving EmberObject.create to a separate method that is not inherited by subclasses, something like:

// @ember/object`

class EmberObject {
  constructor(props) {
    // ..class setup things
  }

  static create(props) {
    deprecate('using EmberObject.create() has been deprecated, please `import createWithProps from @ember/object`', true);
    return createWithProps(this, props);
  }
}

function createWithProps(className, props){ 
    if (typeof props !== 'object') { 
        throw new Error('createWithProps can only be called with {class}, {object}');
    }
    const instance = new className;
    Object.assign(instance, props); 
    return instance; 
}

export default EmberObject;
export createObject;

import createWithProps from @ember/object;

const foo = createWithProps(Foo, {my: 'prop'});

This way, it's explicit that creating a new object with properties is explicitly an ember thing,. but regular new Foo will also work just fine.

[pzuraq]

To be clear, I would say the advisement is not to not use new, it's to not use EmberObject. In general, modern Ember apps can be written without using EmberObject or .create() at all, and we should be recommending that folks actually use native constructors the way they were meant to be used:

// Before
let MyClass = EmberObject.extend();

let foo = MyClass.create({ bar: 'baz' });

// After
class MyClass {
  constructor(bar) {
    this.bar = bar;
  }
}

let foo = new MyClass('baz');

This approach is the way native classes in Javascript are moving in general and what we want to target but, unfortunately, is not codmoddable. Mass assigning properties to the class via Object.assign is not really inline with the way native classes work, and not something we should be recommending, so my only concern with continuing to allow new EmberObject() is that it would actually encourage folks to keep using EmberObject and not actually transition to native classes, like the above.

[oligriffiths]

it would actually encourage folks to keep using EmberObject and not actually transition to native classes, like the above.

Makes total sense. Thanks

[rwjblue]

Discussed this in todays Ember.js Core Team meeting. Some of the take aways from the convo were:

• We should reach out to @wycats for verbiage help in the new EmberObject() deprecation (and deprecation guide)
• Guidance should continue to be "use init", not constructor when extending from a base class that extends from Ember.Object
• We should include a small section in the object model section of the guides explaining why we still advise using init for now.

None of these were blocking concerns, and we are collectively 👍 on moving this into final comment period.

[rwjblue]

There has been no additional feedback since this RFC entered the final comment period, so its time has come...

[mike-north]

As it currently stands, there's no place that can be linked to for a quick and easy understanding of what all this proposal entails.

Can a TL;DR be added to the top of this RFC with super-clear and concise summaries of what it means for consumers?

i.e.,

• do not use constructor(), use init() instead
• do not use new Foo(), use Foo.create() instead

[locks]

Can you update the "How we teach this" section?