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

UI: Computed Utils #5079

Merged
merged 1 commit into from
Jan 25, 2019
Merged

UI: Computed Utils #5079

merged 1 commit into from
Jan 25, 2019

Conversation

johncowen
Copy link
Contributor

@johncowen johncowen commented Dec 10, 2018

This PR includes 2 computed utils, 1 is taken pretty much for the ember source (details in the documentation comments), for the other I'll quote Tom Dale in a recent RFC which he describes what this PR goes some way to solve as 'Dependency Hell'

First, it's annoying to have to type every property twice: once as a string as a dependent key, and again as a property lookup inside the function. While explicit APIs can often lead to clearer code, this verbosity often obfuscates the intent of the property. People understand intuitively that they are typing out dependent keys to help Ember, not other programmers.

Second, people tell us that this syntax is not very intuitive. You have to read the Ember documentation at least once to understand what is happening in this example.

It's also not clear what syntax goes inside the dependent key string. In this simple example it's a property name, but nested dependencies become a property path, like 'person.firstName'. (Good luck writing a computed property that depends on a property with a period in the name.)

You might form the mental model that a JavaScript expression goes inside the string—until you encounter the {firstName,lastName} expansion syntax or the magic @ each syntax for array dependencies.

The truth is that dependent key strings are made up of an unintuitive, unfamiliar microsyntax that you just have to memorize if you want to use Ember well.

Lastly, it's easy for dependent keys to fall out of sync with the implementation, leading to difficult-to-detect, difficult-to-troubleshoot bugs.

Tom proposes the use of decorators to solve this entirely (with caveats), which might be coming at some point soon, in the meantime at least the below is something I find helpful.

The purify function here essentially let's you write:

property: computed(
  'something', 'else',
  function(something, else) {
   return `${something}-${else}`;
  }
)

This does/doesn't let you do certain things:

  1. It doesn't stop you having to type the same things twice, but it does let you refer to them as you'd like to in your pure function. (I might like to call the something property anything in my function, I just rename the argument here)
  2. If you refrain from using get and keep your computed function pure, it goes some way to stop the 'dependency hell'.
  3. We are typing out the dependent keys here because we need to use them not to 'help Ember', if you miss one out you are more likely to notice/know about it. The dependent keys are less likely to fall out of sync with the implementation.
  4. It's intuitive and familiar. You pass some arguments in and return something made from those arguments, a nice pure idiomatic function.
  5. It uses idiomatic ES5, (but you could also use arrow functions or whatever here)

There are no tests to accompany this yet at least:

  1. The factory function is taken straight from the ember source, and is one of those things that is hardly worth testing.
  2. I ummed and ahhed about writing tests for the purify function and I could do at this stage if anyone feels strongly that I should at this stage. Again its one of those functions that is hardly worth testing, but more so than the factory function. See the comments regarding support deeper properties, if I was to implement that I would probably accompany that with some tests as they then would be more worthwhile.

1. The factory is taken from the ember source, but makes it more
reusable
2. Purify converts conventional ember `computed` into a pure version
@johncowen johncowen added the theme/ui Anything related to the UI label Dec 10, 2018
@johncowen johncowen requested a review from a team December 10, 2018 14:33
@johncowen
Copy link
Contributor Author

Just had a thought this morning, the purify thing enables using default arguments which might be nice at some point?

property: computed(
  'items',
  function(possiblyEmpty = []) {
   return possiblyEmpty;
  }
)

@johncowen johncowen added this to the 1.4.1 milestone Dec 17, 2018
@johncowen johncowen modified the milestones: 1.4.1, 1.4.2 Jan 16, 2019
@johncowen johncowen modified the milestones: 1.4.2, Upcoming Jan 25, 2019
Copy link
Member

@hanshasselberg hanshasselberg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:

@johncowen johncowen merged commit 8923882 into ui-staging Jan 25, 2019
@johncowen johncowen deleted the feature/ui-computed-utils branch January 25, 2019 12:28
johncowen added a commit that referenced this pull request Jan 28, 2019
1. The factory is taken from the ember source, but makes it more
reusable
2. Purify converts conventional ember `computed` into a pure version

This commit only adds new files that could be used further down the line
@johncowen johncowen mentioned this pull request Feb 8, 2019
johncowen added a commit that referenced this pull request Feb 21, 2019
1. The factory is taken from the ember source, but makes it more
reusable
2. Purify converts conventional ember `computed` into a pure version

This commit only adds new files that could be used further down the line
johncowen added a commit that referenced this pull request Apr 29, 2019
1. The factory is taken from the ember source, but makes it more
reusable
2. Purify converts conventional ember `computed` into a pure version

This commit only adds new files that could be used further down the line
johncowen added a commit that referenced this pull request May 1, 2019
1. The factory is taken from the ember source, but makes it more
reusable
2. Purify converts conventional ember `computed` into a pure version

This commit only adds new files that could be used further down the line
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
theme/ui Anything related to the UI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants