Doesn't work with async observers

[jakesjews]

I recently had to downgrade my applications ember version from 3.13 due to very slow performance from synchronous observers. When I enabled async observers I noticed that ember-awesome-macros were no longer computing correctly. This repo is a fork of ember-macro-helpers which has failing tests from using async observers.

[enspandi]

We're experiencing same issues. We had just one failing test with "mapBy('...', raw('...))" with "default async observers" disabled. And many more tests failing with "default async observers" enabled.

[kellyselden]

A fix is welcome. I don't have the time to dig in.

[enspandi]

Setting the observers as 'sync' already fixed some issues here:
https://github.com/kellyselden/ember-macro-helpers/blob/master/addon/create-class-computed.js#L54
https://github.com/kellyselden/ember-macro-helpers/blob/master/addon/create-class-computed.js#L126

Like:

... = observer({
  dependentKeys: [mappedKey],
  fn: rewriteComputed,
  sync: true,
});

[enspandi]

@jakesjews Pushed pr at #307. Maybe you can test your app with that?

[jakesjews]

@enspandi I tried something similar and it was functional. Unfortunately it's so slow on newer Ember that our application timed out on its load tests. I ended up replacing awesome-macros with regular computed properties.

[ctjhoa]

For the record, slow perf on observer was planned in async observer RFC
https://emberjs.github.io/rfcs/0494-async-observers.html#drawbacks

[RobbieTheWagner]

@enspandi are you using the workaround from your PR? I am curious how bad the perf issues are. @kellyselden any thoughts on if that PR should be accepted or if you had something else in mind? It would be great to unblock people using this addon from being able to update to Octane.

[kellyselden]

I'm not sure what the implications of that PR are, but it looks simple enough. If the tests pass and it unblocks people, I don't see a problem with merging it.

[RobbieTheWagner]

@kellyselden it seems like tests are passing for everything but Ember 3.8. I'm not sure why exactly. I would definitely like to figure out a solution soon, as we are stuck on Ember 3.14 until this is resolved.

[kellyselden]

In that case, I think with some ember-cli-update magic, we should be able to drop 3.8 support.

[RobbieTheWagner]

@kellyselden do you want to make those updates or would you like me to open a PR for that?

[ctjhoa]

@rwwagner90 FYI, perf are really bad. I'm not even sure this should be considered as a valid fix.
Like @jakesjews we end up replacing awesome-macros with regular computed properties.
We worked on an ember codemod to ease that transition, we probably gonna open source it in a week or 2.

[RobbieTheWagner]

@ctjhoa definitely interested to see that codemod!

Is there no way to get this to work without making the observers sync? @kellyselden any thoughts here? If it makes things unusable in newer Ember, and there is no better fix, I am not sure how to proceed.

[enspandi]

@rwwagner90 Yes, we're using this workaround in production

Our tests suite definitely got slower, but this was in the upgrade from 3.12 to 3.13 - so the cause might not be awesome macros. When using the app we didn't notice a difference 👍 ; Fyi slow test suite was mostly a problem for travis, but locally with running tests in parallel it didn't change much.

But generally we're also slowly transitioning away from computeds and awesome macros and use getters with tracked props. Definitely an improvement with better typescript support as well imo.

[RobbieTheWagner]

@enspandi yeah, we are working on transitioning to tracked as well, but there are sometimes valid use cases for awesome macros still.

[ctjhoa]

@rwwagner90 Here is the codemod to replace macros with regular computed properties.
https://github.com/concordnow/ember-macros-codemod
There are some macros missing and some edge cases the codemod cannot deal with but it will help you to do 80-90% of the work.

[RobbieTheWagner]

@ctjhoa just FYI this codemod did not do anything for me. It only removed imports and did not refactor the computeds.

[ctjhoa]

@rwwagner90 Please open an issue in ember-macros-codemod repo with reproduction samples.
You have the list of the covered macros here with some examples.