Possible 3.15/Octane regression with ember-table addon

[bantic]

We noticed that the release of Ember 3.15 broke the test suite for the Ember Table addon.

The errors we are seeing look like this:

Error: Assertion Failed: You attempted to update `rows` on `<(unknown):ember4815>`,
but it had already been used previously in the same computation.
Attempting to update a value after using it in a computation can
cause logical errors, infinite revalidation bugs, and performance
issues, and is not supported.
            
            `rows` was first used:
            
            - While rendering:
              ----------------
                application
                  index
                    ember-table
                      ember-tfoot
                        this.data-test-row-count

We have an issue open at the ember-table repo: Addepar/ember-table#795
And this is a link directly to the breaking build at Travis: https://travis-ci.org/Addepar/ember-table/jobs/623723306

It may be that we are doing something unusual/unsavory in our code to trigger this, but I thought it might be worth raising attention in case it belies a deeper issue worth looking into, since this was only a minor release and our test suite was green with the previous minor release.

Thanks in advance if anyone has any suggestions about where to go hunting for a fix.

[pzuraq]

We did expect the new assertion to possibly throw in a few existing cases, mostly ones introduced since 3.13 but there could definitely be others. Fundamentally, the assertion is a stronger version of the backtracking rerender assertion, and is triggered because rows was already used before it was updated.

Is rows being updated by test code, or code within Ember Table?

[mixonic]

@pzuraq I'm taking a look. It seems like the stack is fired from https://github.com/Addepar/ember-table/blob/040df201be07fe753ad36a931ce845423d78379a/tests/integration/components/tree-test.js#L20. That line causes a rerender. The original value for rows was at https://github.com/Addepar/ember-table/blob/040df201be07fe753ad36a931ce845423d78379a/tests/helpers/generate-table.js#L154 <- red herring

I don't understand the assertion message (which is probably concerning since I, of all people, should probably be able to understand it). Of course the property is changing after initial render, we are changing it so the rerender will happen.

(update) Ok, I think the phrase "computation" in the assertion is vague or misleading. I think it should say "render"? ...but maybe computation here is apt, I'm not sure, the ember-table code is pretty wild with CP use.

I believe I narrowed it down to:

• https://github.com/Addepar/ember-table/blob/040df201be07fe753ad36a931ce845423d78379a/addon/-private/collapse-tree.js#L786 is where rows on collapseTree is read
• https://github.com/Addepar/ember-table/blob/040df201be07fe753ad36a931ce845423d78379a/addon/components/ember-tbody/component.js#L307 is the set which causes the assertion to fire since rows was already read during calculation of root.

I guess we will try to refactor to avoid it and see if the result is better code. I'll reserve judgement on if I think it is a regression or bugfix until after that :-p

[llunn]

I ran into this also and the error message was indeed difficult to parse.

I was creating a new record in a glimmer component like so, where the route's model hook was also doing a findAll for the same model type.

export default class FormReturnItemComponent extends Component {
  @service store;
  @service moment;

  @tracked model= this.store.createRecord('return');
  // stuff
}

The odd part is that this only occurs when there is initially no results from the model hook (i.e. 0 records returned because none exist). As soon as a record does exist the problem doesn't manifest.

It is still unclear where exactly this assertion is stemming from (the component or the route). In my case, the route is not passing data to the component; the only link between the two is the routes model data and the new record of the same type being created in the component.

@mixonic I discovered two ways to resolve it:

1. Remove @tracked from the model attribute in the component (ramifications of this??)
2. Set the attribute initialization on the next runloop cycle.

Solution 2 looks something like this....

import { next } from '@ember/runloop';
export default class FormReturnItemComponent extends Component {
  @service store;
  @service moment;

  @tracked model;

  constructor(owner, args) {
    super(owner, args);
    next(this, () => { this.model = this.store.createRecord('return');});
  }
  //stuff
}

I don't have enough experience with Octane to advocate for one over the other; perhaps someone else can chime in and provide guidance.

[MichalBryxi]

@llunn be careful with things like:

...
next(this, () => { this.model = this.store.createRecord('return');});
...

It will work for the user, most of the times. Except when it does not. Maybe some flickering will occur, or things being null when you don't expect them to be.

Most importantly it will very likely come back and bite you in the tests.

We were hunting dozens of those errors when tests randomly fail and people just added yet another run.later or run.next and it worked for a while, and then started failing again.

[MichalBryxi]

I was creating a new record in a glimmer component like so, where the route's model hook was also doing a findAll for the same model type.
...
In my case, the route is not passing data to the component; the only link between the two is the routes model data and the new record of the same type being created in the component.

route is not passing data to the component - I don't have a clear picture of the code you have, but right the next part of the sentence seems to contradict this: the only link between the two is the routes model data - What do you mean by this?

I imagine the component is being called as:

<FormReturnItemComponent @model={{@model}} />

If this is the case, then there is a communication between route and component. And since you are mutating data on both in one render loop, you are getting this error.

If you could create a repo with minimal reproducible code, I might be able to tell you how to make it work. cc: @llunn

[pzuraq]

@llunn can you post the full stack trace of the error message that you are triggering? There should be two stack traces included

1. The stack trace in the code where some value was first used
2. The stack trace in the code where the same value was updated, during a single render

This is what the assertion is preventing, and if you provide the full stack trace we may be able to figure out what's causing it. Without it, it's hard to tell what is going on.

[llunn]

It will work for the user, most of the times. Except when it does not. Maybe some flickering will occur, or things being null when you don't expect them to be.

Most importantly it will very likely come back and bite you in the tests.

@MichalBryxi Indeed, thanks; I personally find it very hard to write test code for components.

I believe I was unclear in my previous post, and likely partly due to bad variable names. In my component I have a variable named model, which is a different variable than @model=something passed as a component argument. For example, in the snippet below, the two model attributes are not the same; one is referring to this.model and one is referring to this.args.model. I'm sure you are aware of this, but may have led to confusion based on my bad variable name.

export default class SomeComponent extends Component {
  model = 'local scope';
}

<SomeComponent @model='parent scope'/>

For my component it is simply <FormReturnItemComponent/>, and the sharing aspect was referring to the route querying for a model of type X and the form creating a model of type X, but are otherwise completely independent with respect to data.

@pzuraq What is worse is that I'm now unable to reproduce the problem (after 90 minutes of trying) and thus can't provide a trace. To be honest, my project is new and I have a lot of moving parts at the moment, so who knows where the source was, which is both scary and relieving at the same time! The basic scenario was something like this:

1. On a route, perform a query to the store for a particular model type in the model hook;
2. In a component, create a record in the constructor of that same model type;
3. On the routes template, output the query results;
4. On the component template, bind the new record to form controls;
5. Include the component in the routes template.

I initially thought there might be a race condition going on, where one or more of the following was happening:

1. The routes template was attempting to render a newly created record (from the component) after it has already rendered the record array from the routes model hook (in the same run loop cycle); or
2. The component created the record, which caused ember data to attempt to update the routes model before it was ready (not sure if this is even possible);

If and when I come across this again I will isolate it right away, which may be a good example of how not to do things. ha!

Sorry I couldn't have been more help.

[MichalBryxi]

@llunn maybe this comment in other thread might be of an assistance: Addepar/ember-table#795 (comment)

[nehalbhanushali]

I have had the same issue where ember-source update from 3.14.1 to 3.15.0 causes tests to fail with error on a custom-input component and not ember-table.
This error seems to be associated with the use of classNameBindings in the component.

Attempting to update a value after using it in a computation can cause logical errors, 
infinite revalidation bugs, and performance issues, and is not supported.

`date` was first used:

- While rendering:
  ----------------
    application
      index
       input-wrapper
          this.hasError

[nehalbhanushali]

ember-source 3.16.0 has the fix for this.
#18668

[rwjblue]

Thanks for confirming @nehalbhanushali!

[aarzootrehan]

I am facing a similar error. Not using ember table. The error says :

Uncaught Error: Assertion Failed: You attempted to update [] on <abc@model:someModel::ember509:null>, but it had already been used previously in the same computation. Attempting to update a value after using it in a computation can cause logical errors, infinite revalidation bugs, and performance issues, and is not supported.

[] was first used:

• While rendering:

  application
  stack trace goes here ....
  this.abc

• While rendering:

  application
  stack trace goes here .....
  this.abc
  In my case, this.abc is an ember array initialized in a component with a tracked property.

import { A } from '@ember/array';
@tracked abc = A([])

While the computation happens, this.abc changes it values, intiially from an empty array : [] ,
then while doing
this.abc.pushObject(someObject_here), on this particular line, my code breaks.

Wrapping this.abc.pushObject(someObject_here), in next() works but my code is dependent on this.abc array and so if this push happens in next() , the object consuming this.abc does not works fine.
In other words, next() doesnot solves my problem. Can't use it.

All of this happened when I upgraded my ember-source to 3.16.6. :(

Any other alternative?

[pzuraq]

@aarzootrehan this error means that you are using the array during the render process in some way before you attempt to update it. This could result in, for instance, Ember rendering your array before it was updated, and not updating properly, which is why it is not allowed in general.

The recommendation is to avoid doing this, since it requires you to rerender multiple times. It may mean you have to refactor some code to get things to work in a way where your array is mutated before it is used, but this is generally easier to predict code-wise.

If you think that rerendering is acceptable for this use case, then you can opt-in to a second render pass by scheduling into actions:

schedule('actions', () => this.abc.pushObject());

This is generally not recommended, I really do encourage you to try to refactor your code to avoid this, but you can use it as an escape hatch.

[iamareebjamal]

  get events() {
    return this.args.sessions.map(session => {
      const speakerNames = session.speakers.toArray().map(speaker => speaker.name).join(', ');
      return {
        title      : `${session.title} | ${speakerNames}`,
        start      : session.startsAt.tz(this.timezone).format(),
        end        : session.endsAt.tz(this.timezone).format(),
        resourceId : session.microlocation.get('id'),
        color      : session.track.get('color'),
        serverId   : session.get('id') // id of the session on BE
      };
    });
  }

This code throws "You attempted to update currentState on <open-event-frontend@model:speaker::ember602:4447>, but it had already been used previously in the same computation. Attempting to update a value after using it in a computation can cause logical errors, infinite revalidation bugs, and performance issues, and is not supported.
"

As you can already see that there is no mutation here, ember data may have made some changes internally but I have no control over it. How to avoid it as it is a purely read only code?

[tschoartschi]

I have similar problems 🤔 I do the following (pseudo-code because the real code is pretty much involved):

class Form extends Component {
  @dropTaks
  * createShortId() {
    // ... DO SOME MORE STUFF ...
    // ... REMOVED THIS FROM SAMPLE TO KEEP IT READABLE ...
    const shortId = this.store.createRecord('short-id', {
      referencedId: this.item.id,
      type: SHORT_ID_TYPES.ITEM
    });
    yield shortId.save(); // <-- this .save() breaks
  }
}

Then I get the following error message:

Uncaught Error: Assertion Failed: You attempted to update `_tracking` on `Tag`, but it had already been used previously in the same computation.  Attempting to update a value after using it in a computation can cause logical errors, infinite revalidation bugs, and performance issues, and is not supported.

`_tracking` was first used:

- While rendering:
  -top-level
    application
      protected
        protected.tenant
          protected.tenant.catalog
            protected.tenant.catalog.product
              product-form
                utils/form-for
                  utils/collapsible
                    variations/integration-link-form

Stack trace for the update:
    at dirtyTagFor (vendor-d9433266.js:58431)
    at setter (vendor-d9433266.js:58717)
    at Tag.set [as _tracking] (vendor-d9433266.js:16451)
    at Tag.notify (vendor-d9433266.js:74707)
    at RecordState.notify (vendor-d9433266.js:74920)
    at vendor-d9433266.js:74892
    at NotificationManager.notify (vendor-d9433266.js:91900)

I struggle debugging it because I never use _tracking anywhere neither I use Tag...

Does someone have some tips and tricks how to debug this problem?

[pjcarly]

@tschoartschi what version of Ember are you running?

I am getting the exact same error, but not with the ember-table addon, it is our own code. Nowhere are we using _tracking or Tag whatsoever. And your reply here is the only documented result of someone experiencing the same error.

[tschoartschi]

@pjcarly I needed to add yield Promise.resolve(); as the first statement in the function (in the ember-concurrency task). I can not remember how I found out about this but I think @NullVoxPopuli gave me the tip.

Currently we are on "ember-source": "3.28.1". I don't know if we can remove the yield Promise.resolve(); already but it works now and I'm happy 🙂

[NullVoxPopuli]

More info on yield Promise.resolve() here: NullVoxPopuli/ember-resources#340 (comment)

[pjcarly]

Thanks for the info. Unfortunately I doubt it has anything to do with a store.query in my case.

I'll first have to understand what the error is and then investigate what is going on in my codebase.

I have a hunch it has something to do with ember-data-model-fragments