Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[preview] Cache and share preview snapshots between all subscribers #612

Merged
merged 5 commits into from
Feb 27, 2018

Conversation

bjoerge
Copy link
Member

@bjoerge bjoerge commented Feb 25, 2018

This is a refactor of the preview machinery that enables sharing of preview snapshots among all components that previews fields documents.

Previously, each component/part would have its own version of the document that it previewed, this caused a lot of double fetching, delayed UI updates, etc.

The major change introduced by this PR is that now all parts of the system that requests to preview fields of a document share the same cached representation of the document. For example, if ComponentA wants to preview the fields fieldA and fieldB on document with id doc-123, a cache entry is made for the document with id doc-123, and we fetch and subscribe to changes in fieldA and fieldB of doc-123. Later on, ComponentB may want to preview fieldB, and fieldC on the same document. Now we only fetch and subscribe to changes in fieldC of doc-123. The cached snapshot is shared between both components. If both of these components unsubscribe, we shut down the listeners and no longer update the cached snapshot. However, we still keep the cached snapshot in memory for later. This comes in handy if e.g. ComponentA re-subscribes to the same document/fields. We then immediately emit the last previous known snapshot, while fetching (syncing) the fields again and subscribing to changes in the background.

I also renamed a few functions/files and added a few deprecation warnings to the public API. This API is AFAIK only used by us, and not documented so it shouldn't matter much, and the warnings are there mainly to make it easier to spot our own internal usage.

There is still room for improvements code-wise here, but I've tested this branch quite extensively and feels pretty confident that it works and doesn't contain any regressions (famous last words, I know 😅).

Copy link
Member

@rexxars rexxars left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. There are a couple of debug console.log statements that's just been commented - should we consider a debug part/module to make it easier to include debugging tools without altering code?

(Not related to this PR specifically)

@bjoerge
Copy link
Member Author

bjoerge commented Feb 27, 2018

Should we consider a debug part/module to make it easier to include debugging tools without altering code?

Very good point. I created #616 for a discussion about this.

@bjoerge bjoerge merged commit 0b34988 into next Feb 27, 2018
@bjoerge bjoerge deleted the cache-previewed-docs branch February 27, 2018 12:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants