search

ember-codemods / es5-getter-ember-codemod

34 stars 14 forks source link

Small readme fix #2

Closed PoslinskiNet closed 6 years ago

PoslinskiNet commented 6 years ago

Just to make it consistent. However, not sure do we need to have 2 model examples here (for person definition).

Turbo87 commented 6 years ago

@rondale-sc ☝️

Turbo87 commented 6 years ago

I just noticed that this actually seems incorrect (see https://github.com/rondale-sc/es5-getter-ember-codemod/blob/master/__testfixtures__/es5-getter-ember-codemod/get-on-ember-object.output.js#L3)

rondale-sc commented 6 years ago

@PoslinskiNet @Turbo87 This is intentional. Because we couldn't assume that all objects that use get are Ember Objects or non Ember Proxies with installed es5 getters we only perform the transform if the they are set to variabes controller or route.

See this part of the transform where we assign those names to a variable called typicalEmberAssignment:

https://github.com/rondale-sc/es5-getter-ember-codemod/blob/master/es5-getter-ember-codemod.js#L150

Funny though. I think model should not be among the typicalEmberAssignments. Worth investigating.

PoslinskiNet commented 6 years ago

@rondale-sc thanks for explaining in detail why it was like that 👍

rondale-sc commented 6 years ago

@PoslinskiNet No problem! I think @Turbo87 spent some time reworking the readme so that is less confusing. Thanks for looking into this! :)