ladda-js / ladda

JavaScript data fetching layer with caching
https://petercrona.gitbooks.io/ladda/content/
MIT License
112 stars 16 forks source link

Change hooks and a subscriber plugin #16

Closed LFDM closed 7 years ago

LFDM commented 7 years ago

Hopefully one of the final additions to the core of Ladda - an interface to registers listeners, that are triggered whenever something happens inside of Ladda. This is especially helpful for plugins that want to react to changes and opens up several highly useful practical applications of Ladda.

Technically a third store has been introduced, the listenerStore, which exposes an addListener function, as well as an onChange function, which is passed into the entityStore. Whenever an update or removal to the entityStore happens, this callback is triggered. At the moment a simple Change object of { entity: EntityName, type: UPDATE | REMOVE, entities: Entity[] } is passed to all listeners. This is most likely not enough, as views aren't really well supported yet. Maybe we will end up with passing a list of Change objects in the end - we'll see.

Most importantly, the addListener function is passed to the plugin constructor. It is also exposed on the final API object as __addListener.

The subscriber plugin makes use of this addListener functionality. It patches all READ operations with an additional function called createSubscriber(...args) => Subscriber, where Subscriber is of the shape { subscribe: (successCallback, errorCallback) => unsubscribeFn, destroy: () => void, alive: Boolean } (more about that in a minute).

This led to one important change: The final API object given out of build now also comes with all the initial metadata. While there would be other solutions to this (e.g. placing such additional functionality in its own namespace and only passing this namespace on to the final object), I left everything on the object for simplicity's sake for now.

The callbacks passed to subscribe are called everytime a relevant change is detected through the Change detection described above. Right now it reacts on every change to its own Entity - this can be further optimized - but first and foremost needs to support views. No decision was made on this, because it's not decided yet how the Change detection will deal with views. Callbacks are fed the result of the original apiFn call. There is no magic going on here. Everytime a change is detected, we just invoke this function again (with the arguments originally provided to createSubscriber) - ideally hitting the cache.

Every call to subscribe spawns a new listener - we can unsubscribe by calling the function returned by subscribe. Alternatively a subscriber can be terminated altogether by calling: subscriber.destroy() which also unregisters all present listeners. In addition the boolean field alive on the Subscriber instance is set to false after calling destroy.

All this functionality is nicely unit tested - all information about how this works can be found there.

A practical application of this plugin can be found in https://github.com/ladda-js/ladda-react - the official React bindings for Ladda. Well - technically they don't have any dependency on Ladda, the only contract that needs to be fulfilled is that when you want to subscribe, a Subscriber as specified above needs to be returned.

This repository contains a simple variation of our withResolve higher order component, which now has an additional configuration field subscribe. It's current configuration API is:

// all fields optional
{
  resolve: Map String (props -> Promise),
  subscribe: Map String (props -> Subscriber),
  pendingComponent: Component,
  errorComponent: Component
}

Usage looks like this, given the ladda-subscribe plugin is used:

export default withResolve({
  resolve: { x: (props) => api.x.get(props) },
  subscribe: { user: (props) => api.user.getUser.createSubscriber(props.userId) }
})(Component);

withResolve takes care of properly resolving and updating the component if need be, subscribes and unsubscribes to changes properly.

This is SO powerful!!!

PS: It's a little difficult to use ladda-react at the moment, because ladda does not have it's dist build checked in. I have a local dependency to ladda-cache in ladda-react atm. If you want to check it out, the best thing you can do is an npm install in ladda-react and then copying over the bundle.js file from ladda-cache to ladda-react. This process should be made more pleasant in the future.

coveralls commented 7 years ago

Coverage Status

Coverage decreased (-0.7%) to 99.304% when pulling fa28b63343745b8edc1170e3c413d97ac440c5df on hooks into bfdc1b0b66f8d097dfe940959ff70d2be1839fad on by-ids.

coveralls commented 7 years ago

Coverage Status

Coverage decreased (-0.7%) to 99.304% when pulling 1a1221d23d720e29caf8a45a2383f4720073e7b9 on hooks into bfdc1b0b66f8d097dfe940959ff70d2be1839fad on by-ids.

coveralls commented 7 years ago

Coverage Status

Coverage decreased (-0.7%) to 99.343% when pulling 33751dc1927c96559906f0fab8cf2266092b9735 on hooks into bfdc1b0b66f8d097dfe940959ff70d2be1839fad on by-ids.

LFDM commented 7 years ago

Subscriptions handle views now.

This proved to be an interesting problem, which can be solved in various ways. The big question is: Is Ladda's low level onChange hook aware of views or is this something a plugin should deal with - in our case, the subscriber plugin.

Letting onChange deal with it has the obvious benefit that plugins have an easier time. Let's say we have a user and a miniUser. When we update a miniUser, onChange fires with a single change object for the entity miniUser. Arguably we could fire several changes here (or pass on a list of change objects): One for miniUser and one for user. I decided against this for now. Shooting out several change objects makes the onChange implementation a little more blurry. Should it also take invalidations into account then? I settled for the following:

onChange tells you only about the thing that really changed in the cache, it does not take any indirect change (through a view or an invalidation) into account

While this is a bigger burden for plugins, it's a clearer approach imo.

The subscriber plugin is now view aware. When someone subscribed to a function of a user entity, an update to miniUser will also trigger the callback of the user subscription. (in both directions, child views as well as parents are notified)

It does NOT take views into account in respect to invalidations. I initially thought of doing this. Let's say user invalidates activity. Changing a user will also trigger subscriptions on activity functions. A change to a miniUser will trigger a user subscription - but does it also trigger an activity subscription? (indirectly, because user invalidates it) While I initially implemented it this way, I changed my mind (https://github.com/petercrona/ladda/pull/16/commits/e1d554ddb1fcbb3862fe9f1830b9aa05050a202f), especially because Ladda does not invalidate based on views at the moment to begin with. A change to a miniUser, when only user invalidates activity will not invalidate activity. This is arguably a good feature NOT to implement, because this way Ladda users can define specifically which entities should invalidate each other, because there might be scenarios where only a change to A should invalidate B, while a change to A' (a view of A) does not affect B.

There is probably more real life usage needed to see if this is sufficient as it is - I hope it is.

One additional change to the core plugin was needed to make invalidation scenarios and subscriptions work properly, arguably fixing a buggy (or at least imprecise) behavior: CREATE, UPDATE and DELETE operations acted optimistically on the cache. Changes to the cache were made regardless of the result of a the actual api call. This means that even if an api call would fail, we would still have the cache updated. This kind of optimistic update would be a feature on its own (which we might built soon anyway), but I think it is best not to do this en passant. The current implementation could push the state into an inconsistent state. As we have no real means to recover, I think it is a good chance to disable this behavior for now and revisit the topic of optimistic updates in a separate plugin.

Due to the relation of onChange events, subscriptions and invalidations, I also need to call invalidate before I actually push new updates to the entityStore. Calling put or remove will trigger the onChange hook - which will cause subscriptions to observe the change. If we have not invalidated before this happens, subscriptions cannot work properly and will return stale data that is really already invalidated. The alternative to this would have been to trigger the onChange hook during a later tick (or trigger the subscriptions) on a later tick, but I decided to avoid this for now, as this asynchronous behavior might confuse and/or lead to future problems.

coveralls commented 7 years ago

Coverage Status

Coverage remained the same at 100.0% when pulling 969c9c27c1401b742148098a7009efc111130994 on hooks into bfdc1b0b66f8d097dfe940959ff70d2be1839fad on by-ids.

coveralls commented 7 years ago

Coverage Status

Coverage remained the same at 100.0% when pulling cff35eea6c27465ef1bfd8807e23e6b148708bce on hooks into bfdc1b0b66f8d097dfe940959ff70d2be1839fad on by-ids.

coveralls commented 7 years ago

Coverage Status

Coverage remained the same at 100.0% when pulling 716b51deaeb686c19a044bbc1db62fb69169d408 on hooks into bfdc1b0b66f8d097dfe940959ff70d2be1839fad on by-ids.

LFDM commented 7 years ago

I renamed the plugin - it is not caled subscriber anymore, but observable.

Talked to @Christof yesterday who asked if we're implementing observables with this plugin. We do in fact - it's API was almost completely similar (only change: subscribe returns a Disposable now, instead of a mere unsubscribe function. Call disposable.dispose() to unsubscribe now)

I adjusted the naming accordingly. This is nicer as it will be more recognizable for people coming over from a rxjs background. We can construct nice Angular2 examples with rxjs examples Now too.

I also updated ladda-react's HOC accordingly, which is in fact a nice general purpose higher order component now: it takes any observable (not just ladda ones) and it will just work!

LFDM commented 7 years ago

For even better observable support, we could do something similar as recompose: https://github.com/acdlite/recompose/blob/master/docs/API.md#setobservableconfig

We could allow people to opt-in to a specific Observable implementation, so that Ladda observables immediately work with the library people are using anway.

We would just default to a general observable implementation, as we are doing right now which is just a simple createObservable(...args) => { subscribe: (onNext ,onError) => { dispose; () => {} } implementation.