TAPevents / tap-i18n-db

MIT License
51 stars 17 forks source link

TAPi18n.publish / subscribe never ready in Iron Router 1.0.0 #8

Closed rijk closed 10 years ago

rijk commented 10 years ago

Since the update to Meteor 1.0.0, one of my routes that depends on the @ready() state doesn't work anymore. This seems to be a problem with TAPi18n, because if I change my publish/subscribe methods back to Meteor.publish the problem goes away.

Route:

@route '/home',
    path: '/home'
    subscriptions: -> TAPi18n.subscribe 'courses'
    action: -> console.log 'home', @ready() # never true

Publication (fake):

TAPi18n.publish 'courses', ->
    @ready()

This never prints true to the console. However, this does:

Route:

@route '/home',
    path: '/home'
    subscriptions: -> Meteor.subscribe 'courses'
    action: -> console.log 'home', @ready()

Publication:

Meteor.publish 'courses', ->
    @ready()
rijk commented 10 years ago

So, I did some testing, and the problem does seem to be related to i18nFind as well. When I change this to find, a ready event is submitted, but this doesn't happen when running the exact same queries in i18nFind.

rijk commented 10 years ago

Aww damnit, this was because I was calling it from a non-TAPi18n publish method. This error never shows up by the way, I think it's because you're using Meteor.Error. If you throw a normal Error it does show up in the console, which is desired behaviour I think. By the way 2: Meteor.Error's first argument should not be a numeric code but a string code unique to the error, describing what went wrong (this is what you see in e.g. the console).

Edit: never mind, I read the note about onError in the docs.

rijk commented 10 years ago

After some more digging, it seems to be related to the fact that IronRouter calls the subscriptions function multiple times (if this is a bug or by design I don't know). I inserted a couple logs, and got the following output:

subscribing routes.coffee:18
TAPi18n ready? false tap_i18n_db-client.coffee:120
home route action, ready: false routes.coffee:23
TAPi18n ready? true tap_i18n_db-client.coffee:120
subscribing routes.coffee:18
TAPi18n ready? false tap_i18n_db-client.coffee:120
home route action, ready: false routes.coffee:23
Route dispatch never rendered. Did you forget to call this.next() in an onBeforeAction? iron_core.js:18
TAPi18n ready? true tap_i18n_db-client.coffee:120
subscribing routes.coffee:18
TAPi18n ready? false tap_i18n_db-client.coffee:120
home route action, ready: false routes.coffee:23

So what you can see is it actually turns ready somewhere in the middle, but then the client resubscribes and in the end result it's not ready.

This is the code of my route:

    @route 'userHome',
        path: '/home'
        subscriptions: ->
            console.log 'subscribing'
            TAPi18n.subscribe 'courses'
        action: ->
            console.log 'home route action, ready:', @ready()
            @next()

And the subscription is just calling @ready() and returning, like the one above.

theosp commented 10 years ago

Hi @rijk ,

Thanks a lot for taking the time to report this issue!

I am a bit occupied with the preparations for the Meteor Day in Hong Kong, so I don't have much time to pay attention to it at the moment.

Hope I'll have more time during the weekend.

By the way, I know your pull request is waiting, that's another thing I hope to get to ASAP.

Thanks again, Daniel

rijk commented 10 years ago

So.. this really is a problem with TAPi18n.subscribe. It does not occur when using Meteor.subscribe (see https://github.com/EventedMind/iron-router/issues/1011).

rijk commented 10 years ago

Alright, so I set up a reproduction repo here: https://github.com/rijk/tap-8

What it boils down to is when you use TAPi18n.subscribe, you get:

subscribing routes.coffee:5
home route action, ready: false routes.coffee:8
subscribing routes.coffee:5
home route action, ready: false 

When changing to Meteor.subscribe (and publish):

subscribing routes.coffee:5
home route action, ready: false routes.coffee:8
subscribing routes.coffee:5
home route action, ready: true 
rijk commented 10 years ago

More info in my latest comment.

theosp commented 10 years ago

Got fixed in v0.3.1 thanks a lot for bringing this bug to my attention and taking the time to prepare the sample project!

Sorry I couldn't find time to fix it earlier!

Added you as a contributor on the README .

rijk commented 10 years ago

Cool, thanks for the fix! I tried fixing it myself, but I wasn't familiar enough with the code to figure it out.