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] Regression in 5.6.0 concerning auto-tracking #20657

Open
boris-petrov opened this issue Mar 8, 2024 · 2 comments
Open

[Bug] Regression in 5.6.0 concerning auto-tracking #20657

boris-petrov opened this issue Mar 8, 2024 · 2 comments
Labels

Comments

@boris-petrov
Copy link
Contributor

boris-petrov commented Mar 8, 2024

🐞 Describe the Bug

A couple of my tests started blowing up with the update-after-read assertion after upgrading from 5.5.0 to 5.6.0. Still happens in 5.7.0.

🔬 Minimal Reproduction

Repo

Clone, pnpm install, ember serve, open the page, look at it blow up (in the console) one second later. Downgrading to 5.5.0 fixes the problem. Also, funnily, adding {{#if this.results}}{{/if}} (or anything relating this.results) in parent.hbs also makes it stop blowing up.

😕 Actual Behavior

Autotracking assertion.

🤔 Expected Behavior

No assertion.

🌍 Environment

  • Ember: 5.6.0+
  • Node.js/npm: v21.7.0
  • OS: Linux
  • Browser: Chromium

cc @NullVoxPopuli

P.S. Note this seems to happen because of the style modifier. If it is removed, the code works fine even though it's "touching" the same things. Not sure why...

@NullVoxPopuli
Copy link
Contributor

NullVoxPopuli commented Mar 11, 2024

note that the repro is called ember-resources-bug but ember-resources isn't used here 😅

it's using ember-could-get-used-to-this, which uses a side-effecting callbacks on a class as part of the Helper lifecycle.

@boris-petrov and I talked about this on Discord a good bit, and I think that the lack of error was actually a bug, but what's goofy with this repro is that "the work-around" needs an explanation -- which I haven't looked in to why ends up working out that way -- but seems suspicious.

This is the resource in the repro:

import { tracked } from '@glimmer/tracking';
import { Resource } from 'ember-could-get-used-to-this';

export default class LiveSearchResource extends Resource {
  @tracked results;

  get value() {
    return this.results;
  }

  setup() {
    this.update();
  }

  update() {
    this.results = [this.args.named.counter];
  }
}

which, I would expect that setting tracked data synchronously during create/update would cause a back-tracking assertion, as create and update in HelperManager (capabilities 3.23+) have auto-tracked create/update calls.

The Todos:

  • find an explanation for why {{#if this.results}}{{/if}} gets around the infinite re-render assertion
  • try to find which of the VM changes from 5.5 -> 5.6 caused the change in behavior

@boris-petrov
Copy link
Contributor Author

Ah, sorry for the wrong name! Yes, the "resources" in the name is the generic Ember concept, not the specific implementation of @NullVoxPopuli. :)

We did discuss at length, yes. However, I'm still not convinced that @NullVoxPopuli is right in that particular case. As I mentioned in the conversation with him:

I think the resource is created, setup is called and only then get value is called. Same for update - it is called first and then get value.

Which is a read-after-write which should not trigger the assertion.

Also, there is the fact that removing the style modifier call - the code works fine. Even though the same things happen - the same values get "touched" at the same times. Just no modifier call.

Thirdly, there is this strangeness with {{#if this.results}}{{/if}}.

I might be wrong about all this, of course. You're the Ember gurus. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants