ember-learn / ember-website

The emberjs.com website.
https://emberjs.com/
MIT License
85 stars 184 forks source link

Rewrite controllers in native class (Part 1 of 2) #721

Closed ijlee2 closed 3 years ago

ijlee2 commented 3 years ago

Background

Over the next month or so, we'll upgrade ember-source from v3.20 to v3.24 and ensure that we follow recommended practices in Ember Octane.

Description

In #720, I added unit tests for controllers. These tests, in particular, assert how we transform and sort data before we pass them to a route template.

Now that we have tests, let's rewrite the controllers to use native class.

TODOs

Please update the following controllers that have a low risk of breaking the app. We can rely on unit tests to say that the app functions as intended.

References

If you are unsure about the syntax for controllers in native class, please have a look at the following links in the Ember Guides:

sukima commented 3 years ago

The ESLint is forbidding the use of computed properties in native classes. Considering Ember's Array.sort is different then Ember's @sort it makes this difficult to easily convert especially since recreating @sort within a getter seems like it is adding tech-debt. Any advice?

ijlee2 commented 3 years ago

Hi, @sukima. Good question! I forgot to write down what one would do for @sort.

If you happen to see @computed, you can use a getter without caching. For @sort, can you add a comment with eslint-disable-next-line so that linting will pass?

At a later time, I plan to install ember-composable-helpers to take care of sorting. For now, for this issue, let's work on updating the controllers to native class.

Hope that made sense! Will you be able to help out with completing this issue?

sukima commented 3 years ago

Would a conversion to VanillaJS work? Was able to recreate the sort in three lines of JS.