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

Cannot destructure property 'manager' of '.for' as it is undefined. #20708

Open amk221 opened 1 week ago

amk221 commented 1 week ago

🔬 Minimal Reproduction

Minimal reproduction of an error in ember-modifier

 Cannot destructure property 'manager' of '.for' as it is undefined.

https://github.com/amk221/-ember-modifier-manager/commit/a3765c5e3bcfe9e094f8febcfc971f98b72e0efa#diff-91d02793f5c4fd5e02a401c33509cafcb6fc7613aef16d97afe18f4b4a080d5cR18-R20

🌍 Environment

NullVoxPopuli commented 1 week ago

Does the issue appear in

Thanks!

amk221 commented 1 week ago

ember-source 5.9 = yes, still occurs

gjs = yes, it still occurs in our app, which is gjs with the original error as above. But, in terms of focusing on the minimal reproduction, no, it instead outputs:

Attempted to set the same type of manager multiple times on a value. You can only associate one manager of each type with a given value. Value was (unknown function)
NullVoxPopuli commented 1 week ago

That second error seems like the deps got messed up. Does using pnpm reproduce nhe original error?

amk221 commented 1 week ago

relooking

amk221 commented 1 week ago

You're right.

Here's a gjs branch, with the original error

https://github.com/amk221/-ember-modifier-manager/commit/783298200e28f65e9e636cb5ce5d8ec85056eeab#diff-8b01f9fcdd7fd5370da514614ab30fe4ea2ebfd452dac4b7e4fd3482dc0e5421R55-R57

NullVoxPopuli commented 1 week ago

In the gjs branch, there was an issue. the modifier import was shadowing the keyword.

After the fix (https://github.com/amk221/-ember-modifier-manager/pull/1), the new error is

Uncaught (in promise) TypeError: scheduledUpdateModifiers[Symbol.iterator]().next().value is undefined
    commit runtime.js:3904
    commit runtime.js:3972
    inTransaction runtime.js:3991
    Ember 3
    invoke backburner.js.js:272
    flush backburner.js.js:188
    flush backburner.js.js:344
    _end backburner.js.js:773
    _boundAutorunEnd backburner.js.js:509
    promise callback*buildNext/< backburner.js.js:26
    flush Ember

which is another issue :thinking:

NullVoxPopuli commented 1 week ago

It's possible that one of these caused the issue:

just kidding, these landed in 5.9

NullVoxPopuli commented 1 week ago

ah, no the repro has a problem

NullVoxPopuli commented 1 week ago

Here is the fix:

diff --git a/app/components/foo.gjs b/app/components/foo.gjs
index 2dd2640..56f7ba3 100644
--- a/app/components/foo.gjs
+++ b/app/components/foo.gjs
@@ -23,12 +23,7 @@ export default class Foo extends Component {
   close() {
     console.log("close");
     this.isOpen = false;
-  }
-
-  @action
-  setValue(value) {
-    console.log("setValue");
-    this.value = value;
+    this.value = "foo";
   }

   <template>
@@ -44,7 +39,6 @@ export default class Foo extends Component {
       type="button"
       class="close"
       {{on "click" this.close}}
-      {{on "click" (fn this.setValue "foo")}}
     >
       Close
     </button>

Now... should it work? probably. why doesn't it work? idk.

maybe @chancancode can provide some insights there.

amk221 commented 1 week ago

Thanks for looking!

~I don't think it's the fact that it uses two click handlers, rather, that it updates another tracked property that is an argument to the modifier? Since, you can combine the click handler and the error still happens.~

Anyway, I really would like this to work of course, since the code is 'valid' also don't forget this is an unrealistic minimal production and in reality, one might not be in control to be able to do your suggested fix

NullVoxPopuli commented 1 week ago

Since, you can combine the click handler and the error still happens.

that's not what happens in my PR to your repro :thinking:

Anyway, I really would like this to work of course

yeah, the fact it doesn't work does seem bad

betocantu93 commented 17 hours ago

Im also having this error with ember-focus-trap

Attempted to set the same type of manager multiple times on a value. You can only associate one manager of each type with a given value. Value was (unknown function)

But with FocusTrapModifier, while on host app using ember-eui components