aurelia / templating

An extensible HTML templating engine supporting databinding, custom elements, attached behaviors and more.
MIT License
116 stars 103 forks source link

Uncaught TypeError: Cannot read property 'animatableElement' of undefined #470

Closed enrico-padovani closed 7 years ago

enrico-padovani commented 8 years ago

I'm submitting a bug report

Current behavior:

while removing elements from a repeater I got the error reported in the subject. It happen also that after adding the second item an empty row is added.

The code is fairly simple:

<table>
    <tr repeat.for="file of files">
        <td>${file.FileName}</td>
        <td><button click.delegate="removeFile(file)">delete</button></td>
    </tr>
</table>

and in my vm:

import { autoinject } from 'aurelia-framework';
import { DocumentService } from './document-service';

@autoinject
export class DocumentBox {

    constructor(private documentService: DocumentService) {    }

    files: Array<any> = [];

    removeFile(file) {
        this.documentService.acquireToken(() => {
            this.documentService.deleteDocument(file.Id, () => {
                this.files.splice(this.files.indexOf(file), 1);
            });
        });
    }

}

Expected/desired behavior:

To proper render the dom elements according to the binding status.

jdanyow commented 8 years ago

@enrico-padovani if you put together a reproduction using the aurelia gist we can look into it: https://gist.run/?id=7542e061bc940cde506b

It would be good to confirm you have the latest aurelia bits installed.

enrico-padovani commented 8 years ago

Unfortunately I cannot reproduce it on gist, there are many things involved:

In particular, the DocumentBox custom element is inserted inside a kendo grid detail template then inside a kendo tabstrip. I've tried to move it outside of the grid, but it does the same.

Tomorrow with fresh eyes, I'll try to understand what's going on. But it seems a really subtle error!

EisenbergEffect commented 8 years ago

@enrico-padovani Thank for looking into this. If there's any additional info you can provide or any way that a repro can be created, that would really help. Let's keep the conversation going here until we work out what is going on.

enrico-padovani commented 8 years ago

Ok @EisenbergEffect and @jdanyow rimraf npm_modules => npm install and now It's working great!

Don't know what npm is doing, but sometimes it's messing things up!

EisenbergEffect commented 8 years ago

Thanks for the update. Sorry about that. I'm not sure why npm has some of its issues sometimes either. In spite of that, it's still the best solution.

chris-dickson commented 8 years ago

@enrico-padovani What was the solution here? You updated to the latest Aurelia version and the problem went away? I'm getting this now occasionally as well with v1.1.0.

enrico-padovani commented 8 years ago

It happened again to me also, I've noticed it usually after running npm update. Removing the node_modules folder and reinstalling again had always fixed the problem. Don't ask me why :)

Sayan751 commented 7 years ago

@enrico-padovani Have you found any solution for this? I am also facing the same issue, and rimrafing the node_modules and reinstalling the same didn't help. Any other suggestion?

brenthoneybone commented 7 years ago

I'm having this issue now too after upgrading node and doing an npm update, also completely removed and reinstalled node_modules as stated above with no luck.

Ubuntu 16.04, Node 7.2.0, NPM 3.10.9, Aurelia CLI 0.23.0, Aurelia Bootstrapper 1.0.1, Aurelia Framework 1.0.7, Aurelia Templating 1.1.4, Aurelia Templating Router 1.0.1, Chrome 54.0.2840.100 (64-bit)

After a little digging, the error stems from: SwapStrategies.with which calls - return Promise.all([viewSlot.remove(previousView, true), promise]); ViewSlot remove then looks for that previousView object within its children, however, the children array is empty resulting in the following method call of ViewSlot.removeAt() to have an invalid index resulting in the look up for the view again to fail let view = this.children[index]; // undefined.

Note that this is on initial page load for me so I would've expected there to not have been a previous view. I'll keep digging.

Reverting aurelia-templating-router from 1.0.1 to 1.0.0 solved the issue. @EisenbergEffect I see 1.0.1 was pushed 2 days ago, perhaps there was a regression? I can try set up a new project when I get a chance to try and reproduce it in a clean install however I don't have much time at the moment with strict deadlines on my project.

@Sayan751 FYI - though I see your comment was 4 days ago so it may be a different issue

thomas-darling commented 7 years ago

@EisenbergEffect We're seeing this too - we just updated all Aurelia packages to the latest release and then clean-installed all the packages. As @brenthoneybone suggested, reverting aurelia-templating-router from 1.0.1 to 1.0.0 appear to solve the problem, so it looks like it might be a regression.

chris-dickson commented 7 years ago

@thomas-darling, we're seeing this on aurelia-templating-router 1.0.0.

EisenbergEffect commented 7 years ago

Can someone attach a zip with a sample project in it that reproduces this? Please keep it as simple as possible so we can isolate the cause.

Dawlight commented 7 years ago

@brenthoneybone My findings are the same as yours. previousView seems to be undefined on first site load in 1.0.0, and reverting to it solved the problem.

salminio commented 7 years ago

we are using aurelia-templating-router 1.0.0 and seeing this. App is too large to easily reproduce, but it does seem to be around dynamically compiling custom elements.

Dawlight commented 7 years ago

@salminio I'm not using any custom elements at all, just standard components and router views. Just hit the same problem while deploying my application, which worked fine while running locally on the webpack dev server.

ben-girardet commented 7 years ago

@EisenbergEffect @jdanyow

I'm running into the same issue since last update. I've put together an mini project (attached zip) to reproduce the issue.

The line of code that triggers the error is when I try to splice an array binded to a repeat.for in the file src/resources/elements/messages.js line 74.

Let me know if I'm doing something wrong or if this is a bug ? Thanks !!

splice.zip

EisenbergEffect commented 7 years ago

@jdanyow Are you able to look into this issue? Repro is attached immediately above.

jdanyow commented 7 years ago

Looks like we need to coerce indexes to an integer in the array observation code.

In @ben-girardet's sample there's this:

  closeMessage(messageId) {
    let index = -1;
    for (let i in this._messages) {
      if (this._messages[i].id === messageId) {
        index = i;
        break;
      }
    }
    if (index !== -1) {
      // bug is here, when splicing the _message array
      this._messages.splice(index, 1);
    }
  }

this._messages is an array. Using for (let i in this._messages) { means i will be a string (eg "0" instead of 0). This is breaking the code that computes the splices from the change records.