Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug] @foo !== this.args.foo (double helper eval) #19140

Closed
Herriau opened this issue Sep 18, 2020 · 9 comments
Closed

[Bug] @foo !== this.args.foo (double helper eval) #19140

Herriau opened this issue Sep 18, 2020 · 9 comments

Comments

@Herriau
Copy link
Contributor

Herriau commented Sep 18, 2020

🐞 Describe the Bug

When a helper expression is used as the value for a given arg at the instantiation site of a Glimmer component, then said helper expression is evaluated twice, provided that said arg is consumed both through @someArg and this.args.someArg on the implementation side of said component.

🔬 Minimal Reproduction

Ember twiddle here.

app/components/my-component/component.js

import Component from '@glimmer/component';

export default class extends Component {
  get foo() {
    return this.args.foo;
  }
}

app/components/my-component/template.hbs

<p>@foo.label = {{@foo.label}}</p>
<p>this.foo.label = {{this.foo.label}}</p>

app/helpers/make-foo.js

import { helper } from '@ember/component/helper';

let i = 1;

export default helper(function makeFoo() {
  return { label: `#${i++}` };
});

app/templates/application.hbs

<MyComponent @foo={{make-foo}} />

😕 Actual Behavior

Displays:

@foo.label = #2
this.foo.label = #1

🤔 Expected Behavior

Should have displayed:

@foo.label = #<some-integer>
this.foo.label = #<same-integer-as-above>

🌍 Environment

  • Ember: 3.20.5
  • Node.js/npm: 10.19.0
  • OS: darwin x64

➕ Additional Context

We have a number of helpers that return object references as opposed to primitive values and these object references in many scenarios matter just as much as the properties they carry. For example the reference itself matters when added to / queried against (Weak)Map, (Weak)Set, or simply when performing === or Object.is comparisons. As a result of this bug, on the one hand we have our component's template dealing with one object reference, potentially passing it down to child components, and then passing it back up to action handlers on our JS class, and on the other hand we have our JS class dealing with a wholly different object reference (both objects effectively carry the same properties).

There are certainly a number of scenarios where I can see this discrepancy as harmless but I can also think of quite a few where this effectively breaks data-down in a big way.

@Herriau
Copy link
Contributor Author

Herriau commented Sep 18, 2020

A (possibly) related bug:

Ember twiddle here

app/components/my-component/component.js

import Component from '@glimmer/component';

export default class extends Component {
  get isFooEqualToFoo() {
    return this.args.foo === this.args.foo; // false 🤷‍♂️
  }
}

app/components/my-component/template.hbs

isFooEqualToFoo = {{if this.isFooEqualToFoo "true" "false"}}

app/templates/application.hbs

<MyComponent @foo={{hash label="foo"}} />

@pzuraq
Copy link
Contributor

pzuraq commented Sep 18, 2020

I believe this may be fixed in the latest Ember release, can you check? Either way I would not have expected this to be the behavior in 3.20. The bug with hash is a different bug, hash goes through a separate system than user defined helpers. We unified this system moreso in 3.21 or 3.22 I believe so I think it should be fixed too.

@Herriau
Copy link
Contributor Author

Herriau commented Sep 18, 2020

@pzuraq I checked that this is still an issue in 3.21, as well as 3.22.0-beta.3 (even though the helper expressions seem to be lazily evaluated in the latter, they're still evaluated twice and this.args.foo on the JS side still gets a different value than @foo on the htmlbars side).

@boris-petrov
Copy link
Contributor

boris-petrov commented Sep 30, 2020

I'm also seeing something similar (on 3.21):

<SidePanel @heading={{foo "something"}}>

And in side-panel.hbs I have {{@heading}} and that creates (and preserves) two instances of the foo helper.

P.S. I wouldn't notice that unless it was "bad". We do a bunch of stuff in this helper so the performance suffers and also memory leaks because of all the side-created stuff by the helper.

@pzuraq
Copy link
Contributor

pzuraq commented Sep 30, 2020

@boris-petrov are you accessing it via @heading AND this.args.heading, like the other report? Just want to confirm, I tried starting on this one and it is very deep I think

@boris-petrov
Copy link
Contributor

boris-petrov commented Sep 30, 2020 via email

@pzuraq
Copy link
Contributor

pzuraq commented Sep 30, 2020

That would be very helpful, that seems very strange and different from the original report for sure.

@boris-petrov
Copy link
Contributor

@pzuraq - here you go. It was too simple, not sure if I've made a mistake somewhere. There are two console-log's for a single call to the helper.

@pzuraq
Copy link
Contributor

pzuraq commented Dec 4, 2020

This should be fixed on Ember master! It was fixed by #19258. Please let me know if this is still an issue!

@pzuraq pzuraq closed this as completed Dec 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants