Meteor-Community-Packages / meteor-autocomplete

Client/server autocompletion designed for Meteor's collections and reactivity.
https://atmospherejs.com/mizzao/autocomplete
MIT License
350 stars 109 forks source link

autocomplete constructor is not dealing with reactive variables properly #30

Closed KyleDFulton closed 10 years ago

KyleDFulton commented 10 years ago

Likely, this is not an autocomplete issue, but more of a question. Using meteor 0.8.0 and autocomplete 0.3.0, I am running into a strange situation where my autocomplete gets constructed on spacebars more times then init gets called. Deps is firing my spacebars template multiple times -- the last time, the render is not called on the most recently constructed autocomplete objects, leaving the autocomplete empty.

There are no errors. It does appear to be a race condition... If I put a breakpoint in the defs.js file within the meteor packages on the 'pendingComputations.push(this);' line, the second set of constructors are not invoked and the previously constructed and initialized/rendered autocomplete inputs are hooked up.

Does anyone have any idea about what could cause this behavior?

template use below:

                    {{> inputAutocomplete value=User settings=usersettings id='user' class='input-large' placeholder='please type...'}}

user settings below:

            Template.edit_communityGroup.usersettings = function() {
                return {
                    position: "bottom",
                    limit: 5,
                    rules: [
                        {
                            token: '',
                            collection: Meteor.users,
                            field: "emails.0.address",
                            template: Template.userPill,
                            callback: userSelected,
                            subscription: 'allUserData'
                        }
                    ]
                }
            };

Thanks, Kyle

KyleDFulton commented 10 years ago

also, I have two autocomplete inputs on the same page.

mizzao commented 10 years ago

What do you mean by 'init is not called'? What init are you referring to?

I think the issue is caused by the fact that you are passing a reactive value User into the template. I don't think I've tested this behavior with autocomplete, and Blaze works in weird ways. It's probably not caused by the two inputs.

Could you create a demo app so that I can debug this? Just factor out the part with the autocomplete and provide some way of changing the user value. Meanwhile you can test it or use as a workaround without value=User and it should work normally. If that fixes it, then this is indeed what we'll need to dig into further.

If that's not the problem, then I don't understand what the following means; could you explain it more concisely?

the second set of constructors are not invoked and the previously constructed and initialized/rendered autocomplete inputs are hooked up

KyleDFulton commented 10 years ago

Thanks a lot.

I am referring to the init function in template.coffee that gets hooked up to the render callback.

# Set nodes on render
init = ->
  console.log('AutoComplete inited'); // I added this
  @data.ac.element = @firstNode
  @data.ac.$element = $(@firstNode)
  @data.ac.tmplInst = this

Template.inputAutocomplete.rendered = init
Template.textareaAutocomplete.rendered = init

the inputAutocomplete template is nor rendered the second time. I have the editor bound to a reactive datasource. After I insert the record, meteor keeps the client local value. Everything gets hooked up. After mongo accepts the value, it pushes the server copy of the record back to the client and tries to re-create the autocompletes, but the render never gets called. .
I will try removing the value=User and see. And I will deploy my app to meteor and give you a url.

Thanks a lot.

Kyle

mizzao commented 10 years ago

The render is only called once in Blaze (meteor/meteor#2001 and meteor/meteor#2010). We don't have a good way to deal with reactive updates to data passed to the rendered callback right now.

I recommend you take reactive variables out of the autocomplete instantiation for now, until Meteor resolves some issues that allow us to use them properly in UI components.

P.S. Please fence in your code blocks as I have done for you. Makes things much easier to read.

KyleDFulton commented 10 years ago

I refactored to handle the db insertion asynchronously and only put the record into edit mode after the callback was invoked. That took care of it. Thanks.

One minor suggestion to allow better reuse of functions. please pass the rule along with the doc when invoking the rule callback:

rule.callback?(doc, rule) # Notify that the item has been selected

mizzao commented 10 years ago

I'm going to keep this issue open so that we can see if other users have issues passing reactive variables to autocomplete and to keep it on the list of things to fix.

KyleDFulton commented 10 years ago

and also the element for context.

That is fine -- your issues ;-).

mizzao commented 10 years ago

Mind putting the callback suggestion in a separate issue? It's going to get lost in this thread for sure.

KyleDFulton commented 10 years ago

will do

Neobii commented 10 years ago

It seems that if you just change the data-context AutoComplete is in, it causes autocomplete to stop working.

mizzao commented 10 years ago

Yep, we don't support dynamic changes to data contexts right now. Hopefully this will be fixed with updates to Blaze.

In the meantime, mind letting me know what you're changing in the data context?

Neobii commented 10 years ago

I'm not actually using the data for anything in the autocomplete.

{{#with object}}
<input type="text" name="utilityType"/>
<input type="date" name="expDate"/>
{{> selectUtilitySuppliers}} <!-- this is the template with the autocomplete form field  -->
{{/with}}

So, on the 'change' events, I update the data instead of having save buttons that both dismiss the form and save all the form fields at once.

mizzao commented 10 years ago

@Neobii, can you show the contents of the selectUtilitySuppliers template in addition to its parent? What are you passing in as arguments to the autocomplete helper?

https://meteor.hackpad.com/Blaze-Proposals-for-v0.2-hsd54WPJmDV is tracking API changes that may be able to help us resolve this problem, whatever its cause.

mizzao commented 10 years ago

@Neobii, can you confirm that when it "stops working", it shows the same errors as posted in #35?

Neobii commented 10 years ago

It doesn't have that error, but I'm going to try to pass in the data context in the helper like {{> autocompleteTemplate dataContextINeed}} instead of doing {{#with dataContextNeeded}}{{> autocompleteTemplate}}{{/with}} to see if that makes it work.

Neobii commented 10 years ago

So, that doesn't work, the only thing that happens is the autocomplete template doesn't show up. I'll make a reproduction for you.

mizzao commented 10 years ago

@Neobii and @KyleDFulton, the latest commit c79402f910c0b5ae79abb6a8f0d06951d861a076 gives a workaround for this until the new Blaze Component API is released. Can you try it with your app and let me know if the errors persist?

mizzao commented 10 years ago

Although this is fixed in 0.4.4, I'm keeping this issue open so we can have a better long term solution that doesn't involve re-rendering.

However, it doesn't even appear that things are really "re-rendering" in Blaze. For example, the contents of the input are preserved even in the example app given in #35 across a re-render.

mizzao commented 10 years ago

@Neobii and @KyleDFulton: This issue is fixed on the blaze-refactor branch for Meteor 0.8.3. Autocomplete components now support changing all inputs reactively except for settings.

Please give it a try and feel free to comment in this thread if there are still issues.