Fix deprecations from deprecation-workflow (and probably remove it at all)

[stukalin]

We have a plugin ember-cli-deprecation-workflow which allows us to hide some deprecation messages. However if we want to be able to update Ember to the v3 we'd better fix those deprecations.

Some of them are brought the project dependencies, so if there aren't new version of those dependencies let's at least identify them at list them down.

If all deprecations will be fixed I suggest removing the plugin at all.

• ember-getowner-polyfill.import (should be fixed in Replace ember-validations #1382)
• ember-metal.binding
• ember-views.did-init-attrs (should be fixed in Replace ember-select-list with em-select #1369)
• ds.serializer.private-should-serialize-has-many (should be fixed with the ember-pouch update)

[tangollama]

Thx. Typed or dictated imperfectly on my iPhone (717.385.9970)

…

On Feb 15, 2018, at 9:47 PM, FrankieDickinson ***@***.***> wrote: I will attempt to fix this. First time contributor. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or mute the thread.

[internets]

@stukalin I've been doing a lot of 2.18 updates for 2.x ember apps lately, I'd like to take this on. Ultimately I'd like to take on #1365 as well, but doing this first should make the PRs more manageable. Would you be open to marking #1365 as in progress as well to prevent some potentially ugly merge scenarios?

It looks like #1369 and #1366 are in progress and will be addressing some aspects of this PR. If those turn out to be non-trivial, I can wait for them to land on master and rebase.

[stukalin]

@internets thanks for taking this over. I marked these issues as in progress. Dunno why I can't assign them on you.

[internets]

No problem, happy to contribute!

I believe you can't assign them to me because github requires me to have push access to the repo. Not a popular limitation :) The in progress label should be enough, thanks!

[alexanderbrown21]

@internets feel free to work on #1365 :). It's worth someone with experience of doing existing migration to touch. i'm afraid don't have much free time as can at the moment.

[internets]

@alexanderbrown21 thanks!

@stukalin I've made some progress which can be viewed here: master...internets:1370

Before I open a PR I wanted to go over some notes and questions.

ember-getowner-polyfill.import

As expected this was purely package updates. There is one stubborn package, ember-validations. As you're probably aware, ember-validations is essentially unmaintained. There's a PR that's over a year old that is unlikely to ever be accepted: DavyJonesLocker/ember-validations#442. The way I see it there are two options: fork or replace. I've had success replacing ember-validations with ember-cp-validations. It's less obtrusive than switching to dockyard's replacement, ember-changeset + ember-changeset-validations. I'm not familiar enough with the codebase to give a real LOE for a replacement, but some quick codebase sleuthing points towards medium effort :)

ds.serializer.private-should-serialize-has-many

The pouchdb update did the trick, but results in an intense amount of deprecation warnings:

db.type() is deprecated and will be removed in a future version of PouchDB" deprecation warnings

The offender is worker-pouch, which is operating on a fork under @jkleinsc. If the fork is still necessary, I think the upstream changes need to be merged or at least pouchdb-community/worker-pouch@b4b0eed needs to be cherry-picked to fix this. I am not familiar enough with the pouchdb ecosystem to make an educated decision on that. If it's easy for John to update the fork I'm happy to update it for this PR, if it needs more work maybe a separate PR is better.

Additionally, a new class of warnings come up when running tests. e.g.

WARNING: The view photo_by_visit is out of date. Please update the pouch dump admin to the latest version of photo_by_visit

I'm not familiar enough with pouchdb to make sense of that warning. Looking for some help with that :)

Notes

• inventory-location-picker.js is not glimmer-friendly (back-tracking rerender), the locationPickers CP causes state change during render. A good rule of thumb is to never modify component state in a CP getter. In order to limit the scope of this commit, the call to the code that changes rendered state has been scheduled to run once after the current runloop. It's not efficient, but it's more efficient than it was and avoids the glimmer assertion failure. I suggest a separate ticket to refactor the component to no longer need the scheduleOnce('afterRender', ...)
• I'm not using the const { ... } = Ember destructuring format for these commits because it's not clear what is preferred in the codebase. When we do the Ember 2.18 update we'll have to use the module import syntax across the board, so I kept it simple.
• Are you open to removing package-lock.json? In my experience maintaining yarn.lock and package-lock.json can result in dependency skew between people using npm and people using yarn which diminishes the usefulness of lockfiles. If package-lock.json must be kept, are you aware of any tools for generating them from yarn lockfiles?
• ember-sinon was updated as a result of other package updates, this results in some deprecation warnings from ember-sinon-qunit. updating ember-sinon-qunit will require some updates to abstract-report-controller-test.js and language-preference-test.js. I can do a separate PR unless you'd like it in this one.

[stukalin]

@internets wow, that's impressive amount of research! My comments are below.

ember-getowner-polyfill.import

I'd say let's go with this ember-cp-validations. Looks like it's being actively maintained. Let's maybe create a separate task for that? There we can do a small research if the library completely suits our needs.

The offender is worker-pouch, which is operating on a fork under @jkleinsc.

I went to the base repo just to have a look around and found out, that the owner did a new release v2.1.0 2(!) days ago. This release includes all the changes @jkleinsc did in his fork. So, that means we can switch over to the base package now.

Additionally, a new class of warnings come up when running tests. e.g.

I believe this is related to the way our test data files were created, so that means only tests are affected. Not 100% sure though.

I suggest a separate ticket to refactor the component to no longer need the scheduleOnce('afterRender', ...)

Please, create.

When we do the Ember 2.18 update we'll have to use the module import syntax across the board, so I kept it simple.

Yeah, let's stick to 2.18 style a bit.

Are you open to removing package-lock.json?

Good point. I added the lock file there because I thought it would be a nice practice and everything. However I see that it doesn't make much sense since the greenkeeper doesn't update it and now you're pointing that it could be confusing for the yarn people. On the other hand it is a good practice to have a lock file. So my idea was to do the following:

1. remove package-lock.json (apparently yarn.lock too) since our travis uses node v6
2. generate the shrinkwrap file
3. install the extension for the greenkeeper which updates the shrinkwrap file
4. and tell the others not to commit shrinkwrap on their own, so it'd be the travis only who will update the shrinkwrap

However now I am not sure if yarn is able to work with the shrinkwrap... Anyways, answering your question, yes, I am completely open to that:)

I can do a separate PR unless you'd like it in this one.

I'd say it's ok here, no problem

[stukalin]

Well, yarn can't work with schrinkwrap. Then.... well.. we could go completely to yarn. Especially if this could help somehow migrate away from bower. That means for sure, we'd need to change that everywhere: travis, docker images etc. But this is a matter of a separate discussion.