Skip to content
This repository has been archived by the owner on Jan 9, 2023. It is now read-only.

Replace Computed Property .property() modifier #1515

Closed
donaldwasserman opened this issue Sep 20, 2018 · 4 comments · Fixed by #1519
Closed

Replace Computed Property .property() modifier #1515

donaldwasserman opened this issue Sep 20, 2018 · 4 comments · Fixed by #1519
Labels
good first issue indicates an issue is good for a first time contributor in progress indicates that issue/pull request is currently being worked on

Comments

@donaldwasserman
Copy link
Contributor

This is a long-lived Ember app, and has some some legacy ember code built in. One of these is the usage of the soon to be deprecated Ember computed property .property() modifier.

The usage in HR looks like this:

propertyName: function() {
  return thing
}.property('tracking_property')

this should be replaced with

import { computed } from '@ember/object';
//later....
propertyName: computed('tracking_property', function() {
  return thing
})

There are many instances of this across the app, so creating a codemod Example may be easier, but isn't required.

@donaldwasserman donaldwasserman added help wanted indicates that an issue is open for contributions good first issue indicates an issue is good for a first time contributor Hacktoberfest 2018 labels Sep 20, 2018
@FergusInLondon
Copy link
Contributor

@tangollama - I can jump on this one today; it looks like a good one to get started on - and should expose me to some of the codebase and workflow.

@FergusInLondon
Copy link
Contributor

FergusInLondon commented Sep 24, 2018

List of references to .property() - gist

@donaldwasserman donaldwasserman added in progress indicates that issue/pull request is currently being worked on and removed help wanted indicates that an issue is open for contributions labels Sep 24, 2018
@donaldwasserman
Copy link
Contributor Author

@FergusInLondon - Great! Thanks for the help!

@FergusInLondon
Copy link
Contributor

FergusInLondon commented Sep 24, 2018

I'll leave this here; it's the comment on the above referenced commit. (Github hasn't updated to reflect the fact that the first two notifications link to subsequently squashed commits - so should 404.)

Hoorah, all references to ...}.property(...) have been removed.

 2018-09-24 22:05:26 ⌚  archbox in ~/OpenSource/hospitalrun-frontend/app
± |issue/#1515-Fix-Computed-Properties ✓| → grep -r "property(" ./

 2018-09-24 22:05:34 ⌚  archbox in ~/OpenSource/hospitalrun-frontend/app
± |issue/#1515-Fix-Computed-Properties ✓| →

To Do Tomorrow

  • Run through - module by module - updating any files that don't import computed(... fn()).
  • Style check - some of the lines where functions require more properties have ended up quite lengthy.
  • Consistent usage of parameters to the callback - i.e some have key, value at the moment, and some don't.
  • Prepare Pull Request for Replace Computed Property .property() modifier  #1515 on the origin repository.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
good first issue indicates an issue is good for a first time contributor in progress indicates that issue/pull request is currently being worked on
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants