Ember 2.15 and ember-codemod based on #176 RFC

[snewcomer]

@toranb Lmk what you think about wrapping scheduleUpdate in a run. I think Ember test errors when doing store.push are telling us to make it runloop aware b/c it potentially might be a good idea. Be great to hear your thoughts.

• upgrade application to 2.15
• ember modules codemod
  - [x] move test/helpers/qunit to test-support/helpers/qunit - NEXT PR
  - [x] remove jquery if possible

[snewcomer]

@toranb I removed it from the commit but wanted your input/thoughts on wrapping scheduleUpdate w/ run. I'm glad you had a test that failed though :)

Unit testing results in quite a bit of overhead wrapping every push in a run. What options do you think we have here to potentially reduce some test overhead?

[snewcomer]

Ran https://github.com/ember-cli/ember-modules-codemod to codemod based on this RFC.

emberjs/rfcs#176

[toranb]

Wow @snewcomer this was a huge effort on your part! Great work my friend! Regarding the addition of ember run - I prefer to avoid any change to how we schedule the work and from what I see you were able to leave that as-is

[toranb]

@snewcomer (reading your comments above again) - was your comment about ember.run related to this method in the store?

scheduleUpdate(type) {
    var recompute = this.get("recompute");
    recompute.addObject(type);
    run.scheduleOnce('actions', this, 'updateFilters');
}

To reduce the overhead I did write a qunit helper that would wrap unit tests with an ember.run (to make it less verbose for the test author). Is that not working/ and or useful for this issue?

[snewcomer]

@toranb ah I totally forgot about that. Thanks for the help!

[snewcomer]

@toranb ah so we probably want to include it in test-support so the consuming app can use it. Added it in this PR.

https://ember-cli.com/extending/#addon-project-structure

[snewcomer]

@toranb this was needed for cases where we assign this.store in the test env.

[snewcomer]

just went with the ember-cli recommendations.

[snewcomer]

added a few test scenarios here.

[snewcomer]

Tested this fork with our app and all is good.

[toranb]

@snewcomer refresh my memory here with test support - what will the import look like for this now that we are exporting it from simple store (assuming we merge it as-is) ?

[snewcomer]

@toranb so it will just be merged with the consuming apps test directory. So import { test } from 'my-app/tests/helpers/qunit'

[toranb]

@snewcomer the only concerns I have are that name (qunit -directly exported from test-support). I'm afraid that naming could be 2 generic and in turn stomp over another at some point (if not already) for users with a good deal of addons/ and test helpers. That fair or no?

Regarding the cleanup method - I'm unclear on how arguments is correctly scoped after you flip'd that to an arrow function. In the original implementation we have a function that wraps the inner run and it's the arguments of that parent func we wanted (using the argz variable name you saw). In the new /and improved version it would seem the arguments used are now for the inner most function (not the parent as originally shown). Is this accurate? Totally open to learn about some magic ES2015 stuff but this didn't look right at first glance so I wanted to get your feedback

[toranb]

@snewcomer also w/ the size of this PR would it be possible to pop off that test-support specifically and land that in a day or so? I'd love to pull this in / version it and release it to ensure everything is g2g for all users ;) at which point we can then do a tiny PR for test support

let me know if you are cool w/ this boss

note: I'm also trying to confirm ember-cli v2.4.3 users (who might also be stuck on node 4) have no issue taking this new import syntax. In the ember-redux addon we did a full feature bump for this because we decided to explicitly drop node 4/5 support (not sure that is required here - just curious if we will have trouble). You mind spinning up a *new ember-cli v2.4.3 app (with node 4LTS) and taking simple store (fork) for a test drive to confirm it's cool ?

[snewcomer]

@toranb yeah I agree with you. I think I got carried away w/ this PR. Dropped.

• I agree the name qunit might be too generic. What do you think about test-wrapper or run-aware-test?

• With the => it inherits the outer scope (or maybe better to say doesn't define it's own arguments, this, etc). So arguments turns out to be the arguments for the wrapper function, thus being able to remove the intermediate variable argz. Kind of like how we used to do var self = this; to inherit the outer scope. I tested with our app/addon and replaced a bunch of tests with the modified test wrapper and everything seemed to work.

• ember-try tests 2.4LTS and since I deleted the original tests/helpers/qunit.js file, it uses the test-support file and passes. And the .travis.yml is on node 4. Do you think that is enough?

[toranb]

@snewcomer thanks for the quick adjustment here! I took ember-cli v2.4.3 for a spin myself using this new release v5.7.0 and it's all good from what I can see

toranb/ember-pure-components-example@bf45773

Thanks again Scott !! Really appreciate you helping keep this addon up to date!