sergeyt / meteor-typeahead

Autocomplete package for meteor powered by twitter typeahead.js
https://atmospherejs.com/sergeyt/typeahead
MIT License
146 stars 82 forks source link

Typeahead errors when logging out #70

Open benmgreene opened 9 years ago

benmgreene commented 9 years ago

When logging out, I get the following typeahead errors:

Error: Exception from Tracker recompute function:

> Before: 28931ms (diff: 28931ms)
    at usePostMessage (http://localhost:3000/packages/meteor.js?43b7958c1598803e94014f27f5f622b0bddc0aaf:381:12)
    at http://localhost:3000/packages/meteor.js?43b7958c1598803e94014f27f5f622b0bddc0aaf:410:3
    at http://localhost:3000/packages/meteor.js?43b7958c1598803e94014f27f5f622b0bddc0aaf:415:4
    at http://localhost:3000/packages/meteor.js?43b7958c1598803e94014f27f5f622b0bddc0aaf:1086:3

> Before: 31387ms (diff: 2456ms)
    at Array.forEach (native)

---
Error: TypeError: Cannot read property 'name' of undefined

    at http://localhost:3000/packages/sergeyt_typeahead.js?8199dbdaa0384feeb3570910957df28ee08230b4:193:67
    at String.reverseArgs (http://localhost:3000/packages/sergeyt_typeahead.js?8199dbdaa0384feeb3570910957df28ee08230b4:78:28)
    at Function.jQuery.extend.each (http://localhost:3000/packages/jquery.js?dd8bac56f8fd3666d433d2285ae01e52597cc51a:417:23)
    at Object._.each (http://localhost:3000/packages/sergeyt_typeahead.js?8199dbdaa0384feeb3570910957df28ee08230b4:76:19)
    at SearchIndex.tokenize [as datumTokenizer] (http://localhost:3000/packages/sergeyt_typeahead.js?8199dbdaa0384feeb3570910957df28ee08230b4:192:23)
    at http://localhost:3000/packages/sergeyt_typeahead.js?8199dbdaa0384feeb3570910957df28ee08230b4:456:51
    at reverseArgs (http://localhost:3000/packages/sergeyt_typeahead.js?8199dbdaa0384feeb3570910957df28ee08230b4:78:28)
    at Function.jQuery.extend.each (http://localhost:3000/packages/jquery.js?dd8bac56f8fd3666d433d2285ae01e52597cc51a:417:23)
    at Object._.each (http://localhost:3000/packages/sergeyt_typeahead.js?8199dbdaa0384feeb3570910957df28ee08230b4:76:19)
    at SearchIndex._.mixin.add (http://localhost:3000/packages/sergeyt_typeahead.js?8199dbdaa0384feeb3570910957df28ee08230b4:453:19)

I've tried explicitly destroying the typeahead beforehand, using $(".typeahead").typeahead("destroy"), but it doesn't help. I assume the problem is due to the underlying collection disappearing from minimongo and that change reactivity effecting, and breaking, typeahead.

Seen this? Any thoughts? Thanks much.

sergeyt commented 9 years ago

@benmgreene the issue should be fixed if we find a way to stop tracker. I guess the tracker.autorun should return some handle/function to stop tracking. This handle could be associated with typeahead instance and invoked on typeahead destroy event. Please feel free to send PR.

sergeyt commented 9 years ago

@benmgreene I din't find easy way to stop computation (run by meteor tracker) on destroying typeahead instance. Current idea is to replace $.fn.typeahead to intercept only typeahead('destroy') calls to invoke custom dispose function. Is it acceptable solution? It would be great if typeahead has support of destroy event. Now I can patch the typeahead.bundle.js to raise this event.

benmgreene commented 9 years ago

@sergeyt thanks for your suggestions. I spent some time hacking on a solution -- see:

https://github.com/benmgreene/meteor-typeahead/commit/aabe1e8838f6d0ebb2c032c9f76404d9965beda8

This code works, but the problem is in getting it called. If I call Meteor.typeahead.destroy() from the template's destroy (or now onDestroyed) callback, it's already too late -- the autorun's computation has already been invalidated.

The alternative is to call Meteor.typeahead.destroy() in a different template's event. Currently this won't work because Meteor.typeahead.destroy, similarly to Meteor.typeahead.inject, limits the selector lookup to the current Template.instance() if there is one. It's easy enough to provide a bypass from that, but it'd be much cleaner to find a way to call it from the template's destroy/onDestroyed callback. Perhaps there's a way to defer the autorun?

Any thoughts?

benmgreene commented 9 years ago

I went ahead and added the bypass, and it works, but it's definitely not ideal.

sergeyt commented 9 years ago

@benmgreene I think you are on right track. The solution looks good to me. Please go ahead to send PR.