Necessary changes to observer for full Tracker compatibility

[mitar]

(This is all open for debate and I understand that it is previously undiscussed pull request, but it is easier to debate it with concrete code changes available.)

I am working on integration of Vue with Meteor and I would like that Vue could be a drop-in replacement for Blaze. That is almost possible, because architecture is very similar. Vue is also using reactive dependency tracking, observer, while Meteor/Blaze is using Tracker. Now, there is a lot of code written for Meteor which is using Tracker and it would be great if it could just work with Vue. And in fact it is doable. I managed to create a full 100% compatible Tracker implementation on top of Vue's observer. But there are some changes necessary to observer's implementation and some internals have to be exposed.

This pull request is initial version of changes I had to do to get this first version of Tracker implemented, to get discussion rolling. I would really like to see if this or something like this could get merged into Vue. From my perspective changes do not really change how observer is working, and all existing tests are still passing.

A list of changes:

• added support for after flush callbacks
• adding a way to force flushing
• fixed exception recovery for pushTarget/popTarget
• correctly cleanup queue if there is an exception from getter
• exposing some internals (probably too much, API could be cleaner there)

Requesting comments, feedback, suggestions. This is work in progress. I will add tests in the future, after we finalize that this is OK to do and everything else.

Example app. Notice how I can use Meteor ReactiveVar inside computed value directly.

[defcc]

Thanks @mitar , you need to follow the contribution guide when opening a PR. Please refer: https://github.com/vuejs/vue/blob/dev/.github/CONTRIBUTING.md#pull-request-guidelines

[mitar]

@defcc: I thought I did. Which part from the guide I am missing? I will add tests later for new features. I will fix failing tests now. Is there anything else?

[mitar]

Hm, tests are now failing at "should get updated with model when in focus" which seems unrelated to my changes? Flaky tests?

[defcc]

Thanks @mitar , I opened a PR to fix that error.

[mitar]

Tests are passing.

[yyx990803]

Hey, I think this is an interesting approach, but here's why I think it's unlikely to get merged: these changes increases the public API surface and will impact how we can make changes in Vue in the future.

By exposing the internals, it creates a liability for Vue to keep these APIs intact in order to stick to semver (otherwise it would break anything that relies on them, e.g. your Tracker wrapper). However, the scheduler and the reactivity system really is considered internal, and I intend to keep them internal so that we can iterate and make non-trivial changes to them without affecting the public API. There are a few experiments going on currently (e.g. using ES6 Proxy for change detection, slicing scheduler workload with requestIdleCallback) that all may cause changes to how the scheduler/Watcher/Dep system works, but if these internal implementation details become part of the public API, it places a serious constraint on how we can iterate on things in the future.

I do see the benefits here in that it allows the two reactivity systems to integrate more seamlessly, however I'm not sure whether it justifies adding the extra liability.

For now, a fork seems like the best short-term solution to me. In the long run, I will probably refactor Vue's reactivity system so that it can be exposed in a way that is less coupled to the internal details.

[mitar]

@yyx990803, thank you for your answer.

I do understand your position, but I also think that there are some alternatives available here.

I am not sure what is your position of using _ prefixed symbols. But this could be one way to expose those internals in a way to communicate that those symbols are internal and could break at any point and using them requires that consumer makes sure things work and things are integrated correctly. For example, from our side we could simply exactly require specific version of Vue and upgrade it in Meteor package only after a new version is tested. But at the same time, some other developer can try and force a new version much easier than forking our fork and changing things there to get this new version.

Another approach would be to provide a very simple API to register an "observer" module with which one can override default observer code (Dep, Watcher, and queueWatcher). Only the API of those would be fixed, but internal implementation would be left to the module to implement.

[Akryum]

Exposing internals with _ while considering them not part of the public API seems fine to me. And the Meteor integration could pin the version of Vue.

[aadamsx]

@yyx990803, have you made a final decision on this? :)

We in the Meteor/Blaze community need this change in order to move over to your framework! There's a lot of developers over there waiting excitedly for this development!

Can't we compromise on this by using the "_" to expose just the few internals we need? We understand if you decided to make breaking changes to this portion of the API we're on our own. Heck, shut down the said APIs if there is a breaking change and we'll be responsible for updating said APIs and brining them back online once everything passes.

I believe the effort will expose a entire community to your UI framework, possibly increasing the mindshare and popularity!

[mitar]

I love to see the enthusiasm of Meteor community for this. :-) Thanks.

Just to be clear, exposing symbols with _ is not enough, there are also some changes needed the code as shown in this pull request:

• added support for after flush callbacks
• adding a way to force flushing

(BTW, I will add tests for all those changes if we decide to merge this pull request.)

An alternative approach, of providing a way to register a custom implementation of Dep, Watcher, queueWatcher would allow us to simple completely swap the implementation, but of course the downside is that this common code would not be maintained (and especially coordinated!) together, which is a downside. But on the other hand it would not require from you to maintain this part of codebase.

I would also like to point out that some things should probably be merged in any case. I found out that current code is very bad at recovering from any exception, leaving the whole system in an undefined state. You can only reload to recover. This is why I added things like:

• fixed exception recovery for pushTarget/popTarget
• correctly cleanup queue if there is an exception from getter

Meteor Tracker tests are pretty hard on testing those edge cases and this is how I found them.

[jamiter]

If this will be possible, in whatever form, I dare say that Vue will become the default used framework with Meteor. At least for everyone still on Blaze. This would be the most sophisticated and incremental way to start using Vue. Good for both communities!

I started rendering some parts of our app with React. Although React is a great framework, it feels like a mismatch with Meteor. Or tracker, to be more precise.

I truly hope this interoperability will see the light of day!

[mitar]

Resolved merge conflicts with current dev branch.

[aadamsx]

@yyx990803, you added a 'pending' label to this PR. Sorry, but what does this mean for us waiting on this change to integrate Vue into our existing Meteor projects? Do you plan to merge this PR?

Just in case: Blaze has been broken out from Meteor and is now a stand alone project. @mitar is the primary owner/maintainer for Blaze (no longer MDG). He is trying to use Vue as the foundation of a Blaze 2 that will serve as the "default" UI for Meteor (the alternative is React). But without this PR, it will be an impossible task.

@yyx990803, please allow the Meteor community to further migrate towards Vue by accepting this PR and helping @mitar where needed.

Thank you for a super great framework!

[znck]

Why are you not using default import syntax?

L22: import Watcher from '../observer/watcher'
L20: import Dep, { pushTarget, popTarget } from '../observer/dep'

[mitar]

No particular reason. I can change it, if you prefer.

[Akryum]

I find @znck syntax cleaner. 😄

[mitar]

Updated.

[mitar]

I moved forked Tracker into its own repo: https://github.com/meteor-vue/tracker
And also made it easier to use forked Vue: https://github.com/meteor-vue/vue

Now it is easy to try all this in your app: https://github.com/meteor-vue/guide

[mitar]

BTW, if anyone can help here. Do you know how I could configure something to notify me when this pull requests gets conflicting files. Now I see that it again has conflicting files, so branch should be updated.

[aadamsx]

@mitar

Do you know how I could configure something to notify me when this pull requests gets conflicting files.

maybe this can help with the alerts: https://gitnotifier.io/

Also, this software looks comprehensive and might have the feature you're looking for: https://www.realartists.com/blog/ship-20.html

[mitar]

@yyx990803: What is your take on how to integrate Minimongo the best? Now that we have proper Tracker support, it means that if you do collection.find().fetch() inside Vue computed function, it will rerun on every change, as expected. But what should we do if you return collection.find()? Should we do anything smart, or just call fetch() on it and Object.freeze it? We should also make so that Vue uses _id as a default key for Minimongo results (arrays or cursors). Should we specially handle cursors so that Vue observes changes to underlying data without having to compare whole arrays?

[mitar]

I have finished integrating Tracker with Vue. I think the result looks amazing. There are some internals we could continue improving, but I think that interface for developers works well.

To showcase it, I implemented TodoMVC in it. See this file for an example of Vue component using familiar Meteor concepts, like reactivity, subscriptions and methods. (And also some custom packages, to show how they continue working as they worked in Blaze.)

Some instructions how to use it can be found here. After more people tested it and more feedback, we can work on putting this into a package and more reusable form.

Also, this is on purpose very non-MVVM and trying to be as close to what experience with Blaze is. (So subscribing to data, calling methods to modify data on events, without any fancy multi-way data binding.) Those things can be future work.

Comments and feedback welcome. Best place for that is this issue.

This is all possible because of the amazing work by @Akryum! It is just combining things he made in a bit different way.

One potential step (which I am not planing to do personally, but would invite somebody to do) is to convert spacebars compiler to convert Blaze template syntax to Vue templates, in a similar way how Vue allows to use Pug/Jade.

[mitar]

Also see #6079 with some more useful changes which would help make Minimongo integration smoother. (I am still unclear what would be the best way to do that integration.)

[alexandesigner]

Any updates to this issue?

I'm using @mitar's fork, but maybe it would be interesting to have an official integration

[yyx990803]

My previous comment still stands true as we've planned to do a reactivity system overhaul (using Proxies internally) in early 2018. The chance for this PR to be compatible with the upgrade will be slim. If the need for Meteor integration is still in reasonable demand at that point, we can revisit what that entails after the upgrade.

[mitar]

What would be the best way for me to be involved in that system overhaul to help make sure things can work together from the very beginning. So that it is not a later change which as you see now is much harder to get in?

I think that 38 upvotes on this pull request is clear that there is some demand for this.

Also, why not still merge this in as it is now, and then as upgrade happens, also the new system could contain things useful for Meteor?

[yyx990803]

The main issue: the changes are too intrusive, so it is essentially asking the Vue team to take on liability, design constraints and potential technical debt for some need that is entirely induced by a 3rd party system.

In order to get involved in the process, it'd be helpful to have a proper write up of what is fundamentally preventing Vue's reactivity system to work with Tracker's; and instead of directly changing Vue's internals, providing a desired API surface that you'd like to see exposed (even better, test cases of these exposed APIs). That helps us gauge the constraints better and maybe can come up with something that decouples the two systems better.

[aadamsx]

In order to get involved in the process, it'd be helpful to have a proper write up of what is fundamentally preventing Vue's reactivity system to work with Tracker's; and instead of directly changing Vue's internals, providing a desired API surface that you'd like to see exposed (even better, test cases of these exposed APIs).

@mitar do you have any plans on providing the stuff he's asking for above or have you moved on from this effort?

[mitar]

@aadamsx, thanks for reminding me to reply here.

@yyx990803, I think there might be a slight misunderstanding here (of what this pull request is doing, or of what I am trying to achieve), so just to make sure I will summarize it. I ported Meteor's Tracker on top of Vue's reactivity system. Vue's is really a great reactivity system and Tracker was able to be ported to it with almost 1:1 API mapping only. The only small differences are that Tracker has some more hooks to be called (like after each invalidation, allowing to force flushing, small things like that). This PR might look intrusive, but it is mostly just adding such places for hooking in.

So I see two options, the first being this PR:

• Vue reactivity system exposes few more callbacks so that Tracker can be directly built as a API transformation on top of Vue reactivity system (personally, I do not see it as too intrusive, but this is your call, and maybe they could even be made cleaner than I made them)
• Vue makes its reactivity system pluggable, so that user can say, "hey, instead of this default implementation, swap it with this other JS module"; it would be on the second module to assure compatibility with default system

I think based on the feedback you gave here, I think the second approach would be better. Instead of us having to define API space and have to push back and forth on which callbacks Tracker would need and which one you would be OK to provide, you just freely design Vue reactivity system how it is, just allow that somebody can call something like Vue.useReactivity(module) and it would start using the new module instead. And if new module is buggy, it is on their maintainers to make it compatible. Based on PR here I see it that it would be easy to maintain such alternative implementation exposing few more callbacks. And it would be easier than providing the whole fork of Vue.

What do you think of this approach?

Just one reply from before:

we've planned to do a reactivity system overhaul (using Proxies internally) in early 2018

So because I am building Tracker on top of Vue reactivity system, I do not think it is really important how internally you implement it. I am glad that Vue reactivity system has a concept of "user" watchers but beyond that it is just using the external API. See here for details.

[msavin]

I heard the Vue logo is on Meteor.com - it sounds like this is going well :)

[mitar]

@yyx990803, what do you think about ideas I made above how we could make this work for both communities?

[mitar]

@yyx990803 ping. Is there a better channel to communicate about this?

[mitar]

I opened #8884 as a followup with hopes to get something like this into Vue 3.x.