Upgrade ember-modifier to v2

[SergeAstapov]

ember-modifier@2.0.0 has just been released with support for @ember/destroyable which otherwise rises deprecation in Ember.js 3.20+.

Without dependency version bump in ember-in-viewport, an app can't upgrade ember-modifier

[SergeAstapov]

Hi @snewcomer! Do you mind to bump new version of ember-in-viewport once this got merged?

[SergeAstapov]

hmmm, looks like there are some tests failures related to @ember/destroyable API

/home/travis/build/DockYard/ember-in-viewport/ember-modifier/-private/class/modifier-manager.ts: @ember/destroyable does not have a destroy export

[SergeAstapov]

CI is not happy. Logged https://github.com/ember-modifier/ember-modifier/issues/52 I'll update PR once it got resolved

[SergeAstapov]

CI failures with release/beta/canary seem to be unrelated to the changes in the PR as I see exactly same errors in our app when tried to upgrade Ember.js to 3.20:

not ok 19 Chrome 84.0 - [165 ms] - Acceptance | infinity-scrollable: works with in-viewport modifier
    ---
        actual: >
            null
        stack: >
            TypeError: Cannot read property 'get' of null
                at InViewport.unobserveIntersectionObserver (http://localhost:7357/assets/vendor.js:70445:46)
                at InViewport.stopWatching (http://localhost:7357/assets/vendor.js:70476:16)
                at InViewportModifier.destroyWatcher (http://localhost:7357/assets/vendor.js:70174:25)
                at InViewportModifier.willRemove (http://localhost:7357/assets/vendor.js:70197:14)
                at destroyModifier (http://localhost:7357/assets/vendor.js:70796:14)
                at invoke (http://localhost:7357/assets/vendor.js:57808:16)
                at Queue.flush (http://localhost:7357/assets/vendor.js:57698:13)
                at DeferredActionQueues.flush (http://localhost:7357/assets/vendor.js:57905:21)
                at Backburner._end (http://localhost:7357/assets/vendor.js:58479:34)
                at Backburner.end (http://localhost:7357/assets/vendor.js:58165:12)
        message: >
            Promise rejected after "works with in-viewport modifier": Cannot read property 'get' of null
        negative: >
            false
        browser log: |
    ...

[snewcomer]

fyi this PR added a package-lock file.

Regarding the beta failure, it looks like modifier is torn down after the service? It seems like that is why this.registry is null?

[SergeAstapov]

@snewcomer package-lock.json exists in master so I wasn't sure if I should use npm or yarn hence committed updated both package-lock.json and yarn.lock.

Regarding the beta failure - noticed same thing. Maybe we should update tear down logic in service and clean up reference to observerAdmin and rafAdmin once destroy and reset invoked?

[snewcomer]

@SergeAstapov Ah you are right. Lets remove the package-lock. Also I'm ok moving stuff around so teardown works!

[SergeAstapov]

@snewcomer build pass 🎉

[alexlafroscia]

Any chance a release could be cut now that this has been released? I have a number of packages that I’m waiting to update until they depend on ember-modifiers@v2 and this was the last one that needed an update! 😬

[SergeAstapov]

@alexlafroscia I believe this fix is part of v3.8.0 so you should be good to go.

[alexlafroscia]

Ah, right you are! I missed that; the GitHub UI shows the last release being back in 2018 😅

Thanks again for getting this merged!