Change Tutorial examples to use Class Syntax (#540)

[muziejus]

@jamescdavis

[muziejus]

This fixes #540

[jamescdavis]

First pass CR. This looks really good.
Found two issues (missing constructor args and improperly calling parent method).
@classNames is a question. @jenweber ?

[jamescdavis]

Do we also want to use the @classNames decorator? @jenweber

[muziejus]

I've added them. I forgot about this decorator 😃

[ghost]

Not to muddy the waters here too much, but I thought native class syntax should only apply to Glimmer components: https://github.com/ember-learn/guides-source/blob/octane/guides/release/upgrading/editions.md#glimmer-components. Does this seem correct?

[MelSumner]

@pzuraq can you review this bit please?

[pzuraq]

yes, native class syntax applies to everything except classic components. This is because we don't have first class APIs for things like classNames.

[rwjblue]

The recent changes to the component blueprints in emberjs/ember.js to support octane were done before we had decided that we didn't want to show native class syntax with Ember.Component at all.

Would someone mind making an issue over there to get it fixed (if you cc @ppcano he will probably be willing to fix it up...)

[ghost]

created that issue: emberjs/ember.js#17701

[pzuraq]

This is generally looking good. I think the last step we need to do is to convert the classic components to Glimmer components, which should be fairly straightforward (let me know if you need any help with it though!) and to remove any references to ember-decorators is favor of Ember's own first-class decorators

[pzuraq]

yes, native class syntax applies to everything except classic components. This is because we don't have first class APIs for things like classNames.

[pzuraq]

ember-decorators is not an official Ember addon, so we aren't going to have it in any examples. Ember itself does support the following decorators:

import { computed, action } from '@ember/object';
import { inject as service } from '@ember/service';
import { inject as controller } from '@ember/controller';
import * from '@ember/object/computed';

This does mean that we don't have the proper decorators to convert classic components to native class syntax, which is what the core team has agreed on. Everything except class components should always be native classes, and classic components should use classic class syntax.

[muziejus]

Thanks, all. I'll have a look later today and see what I can fix up.

[ghost]

Thank you @muziejus for taking this on!

[pzuraq]

So, we should actually convert this to a GlimmerComponent. You can check out the edition guide to see what the differences are. In this example, we would update the template like so:

<div class="list-filter">
  {{input
    value=this.value
    key-up=this.handleFilterEntry
    class="light"
    placeholder="Filter By City"
  }}
  {{yield this.results}}
</div>

This moves the classNames to be a part of the template, because Glimmer components don't have a wrapping element. We also remove the {{action}} helper from around the action, since it's no longer necessary (@action does the same thing). We would then update the class like this:

import Component from '@glimmer/component';
import { tracked } from '@glimmer/tracking';
import { action } from '@ember/object';

export default class ListFilterComponent extends Component {
  @tracked value = '';
  @tracked results;

  constructor() {
    super(...arguments);
    this.args.filter('').then((results) => this.results = results);
  }

  @action
  handleFilterEntry() {
    let filterInputValue = this.value;
    let filterAction = this.args.filter;
    filterAction(filterInputValue).then((filterResults) => this.results = filterResults);
  }
}

Here we're importing from @glimmer/component instead of @ember/component, which is the new base class. We also have to use this.args to refer to arguments because of this, so we have to update this.filter to this.args.filter in general. Finally, Glimmer components don't have this.set, so we either have to use Ember.set, or convert to tracked props. I went for the latter here.

This PR is really looking great, thanks for being understanding, I know there are a lot of changes and they're hard to work through and not all that transparent yet. This is going to be very helpful in getting all this info out there 😄

[muziejus]

Oh this is great. Thanks. I understand perfectly now, I think. And I'm glad for the clarity about wrapping tags, too, since I'm sick of dealing w/ classNames and tagNames in my component.jses anyway.

[muziejus]

OK, between @pzuraq's responses and what @efx did for the component blueprint in emberjs, I think I've got a general sense of what the goals are here:

• Components come from glimmer (done w/ ad8f9b3 )
• No ember-decorators decorators (those are done w/ 0d45515 )
• Routes can use native syntax and get that from ember (already in place)
• Ember Data stuff I've more or less reverted, because the model blueprint still uses export default DS.Model.extend({... (gone w/ ad8f9b3 )
• Use @tracked decorator & fix up args issues (tk)

[muziejus]

@pzuraq Pinging you again. I think I've gotten it, and I caught a few overzealous mistakes from elsewhere.

[pzuraq]

Awesome job, thanks again for this!

[ppcano]

👏@muziejus

Ember Data stuff I've more or less reverted, because the model blueprint still uses

Ember Data blueprints will also use Native syntax, check out the open PR emberjs/data#5859

[ghost]

@ppcano indeed; I think we are tracking that work in #543. I started that PR in #553.

[muziejus]

Excellent. Thanks for the support, all.

@efx I was going to go back through and fix the Ember Data stuff after I saw @ppcano's message, but now I understand I should not and should simply let this ride and help out again in the future when I can.