emberjs / ember.js

Ember.js - A JavaScript framework for creating ambitious web applications
https://emberjs.com
MIT License
22.46k stars 4.21k forks source link

[Bug] destroyModifier triggers ember-data:method-calls-on-destroyed-store #19447

Open wongpeiyi opened 3 years ago

wongpeiyi commented 3 years ago

🐞 Describe the Bug

Upon acceptance test completion, modifier destroy on app teardown results in ember-data:method-calls-on-destroyed-store warning if the modifier args is a RecordArray

This happens from 3.22 onwards. I believe this happens because teardown results in the modifier calling registerDestructor(state, () => delegate.destroyModifier(instance, state.args)); but the args then accesses the store when store.isDestroying

Not sure if its more an ember data issue and should be posted there instead.

Perhaps related to https://github.com/emberjs/ember.js/issues/19162 ?

🔬 Minimal Reproduction

Might be totally off in my understanding of what's happening, so I came up with a minimal reproduction here: https://github.com/wongpeiyi/modifier-destroy

😕 Actual Behavior

Acceptance test results in deprecation warnings:

DEPRECATION: Attempted to call store.adapterFor(), but the store instance has already been destroyed. [deprecation id: ember-data:method-calls-on-destroyed-store]
        at logDeprecationStackTrace (http://localhost:7358/assets/vendor.js:36451:21)
        at HANDLERS.<computed> (http://localhost:7358/assets/vendor.js:36548:9)
        at raiseOnDeprecation (http://localhost:7358/assets/vendor.js:36478:9)
        at HANDLERS.<computed> (http://localhost:7358/assets/vendor.js:36548:9)
        at invoke (http://localhost:7358/assets/vendor.js:36560:9)
        at Object.deprecate (http://localhost:7358/assets/vendor.js:36516:28)
        at assertDestroyedStore (http://localhost:7358/assets/vendor.js:89833:66)
        at Store.adapterFor (http://localhost:7358/assets/vendor.js:89432:9)
        at Store._findHasManyByJsonApiResource (http://localhost:7358/assets/vendor.js:88143:26)

DEPRECATION: Attempted to call store.findMany(), but the store instance has already been destroyed. [deprecation id: ember-data:method-calls-on-destroyed-store]
        at logDeprecationStackTrace (http://localhost:7358/assets/vendor.js:33734:21)
        at HANDLERS.<computed> (http://localhost:7358/assets/vendor.js:33867:9)
        at raiseOnDeprecation (http://localhost:7358/assets/vendor.js:33761:9)
        at HANDLERS.<computed> (http://localhost:7358/assets/vendor.js:33867:9)
        at invoke (http://localhost:7358/assets/vendor.js:33879:9)
        at Object.deprecate (http://localhost:7358/assets/vendor.js:33835:28)
        at assertDestroyedStore (http://localhost:7358/assets/vendor.js:89633:66)
        at Store.findMany (http://localhost:7358/assets/vendor.js:87901:9)
        at Store._findHasManyByJsonApiResource (http://localhost:7358/assets/vendor.js:87965:21)

🤔 Expected Behavior

Warnings shouldn't be shown. Modifier shouldn't recompute args on destroy?

🌍 Environment

boris-petrov commented 3 years ago

This is perhaps fixed by https://github.com/glimmerjs/glimmer-vm/pull/1276?

wongpeiyi commented 3 years ago

That looks promising, thank you