Use real subscription context

[mitar]

Pull request for #159.

[mitar]

Because I use official code to publish, not accounts-base returning null instead of calling ready() is showing a warning. I opened a pull request to fix that: meteor/meteor#6363

[mitar]

So this seems to work for me. I made it so that the publish context is really the official one. Also much of the official code is reused. I also fixed some small bugs which I discovered (for example, passing values as reference instead of copying it, and not removing fields in changed callback when value is undefined).

I would guess we could go even further and even make our "session" object the official one, and make it so that it keeps internal state of documents and all merging across subscriptions could be reused, we only make it so that when session sends a message, it does not really send a message to the client, but to our copy of client data on the server. So we override the session's sendAdded and similar methods. I think this would make everything much easier and completely compatible with Meteor (with all diffing and merging and stuff done correctly).

[arunoda]

I think this is great. Merge box related stuff goes to a different PR I guess after this.
There's one more we need to do.

There are many apps (and yes accounts package) returns null thinking it'll stop the publication. We need to handle it with by overriding _publishHandlerResult.

On there, we need to convert null into [].

[mitar]

There are many apps (and yes accounts package) returns null thinking it'll stop the publication.

But how does Meteor handle this? How does it know that client-side does not have to wait for .ready() and it can render things?

[mitar]

BTW, publishHandlerResult is a separate function only in development branch. This was a pull request I made. So now it is easy to fix this, but it will not work for older versions of Meteor where publishHandlerResult is part of the runHandler.

[arunoda]

@mitar Okay got it. (yeah, the code looks fresh to me).
Then we may need to override handlers, but that's hacky.

In the accounts case, it's a universal publication and it has no meaning of ready. Some users do the similar in universal pubs and some don't do ready checks.

This is a pretty big issue if done in the FR related code. (Since this leads to 500ms delay in the server).

[mitar]

OK, I will address this by wrapping the handler itself.

[arunoda]

And then, you may need to wrap, Meteor.publish for runtime and publication loaded after Fast-Render.

[mitar]

BTW, this is also something which was removed in the process. I do not know if this is problematic.

[arunoda]

That's fine. I assume there's no app use smart-collections these days :D

[mitar]

I looked into this a bit more. In fact, universal subscriptions do not even have ready and stopped state. So this is in fact a completely arbitrary choice for us when we decide it is "ready".

I would propose that instead of checking for null, we just assume all universal subscriptions are ready immediately after their handler exits.

[arunoda]

@mitar Okay. That's a good idea.
For other cases, we'll add some note here: https://github.com/kadirahq/fast-render/blob/master/lib/server/context.js#L102

[mitar]

Fixed.

[mitar]

I also added this now.

[arunoda]

Awesome.
This looks great to me. I'll take this in.

[arunoda]

Merged and released as v2.13.0