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

Introducing the PresenceStore #330

Merged
merged 4 commits into from
Nov 2, 2017
Merged

Introducing the PresenceStore #330

merged 4 commits into from
Nov 2, 2017

Conversation

simen
Copy link
Member

@simen simen commented Nov 2, 2017

Implements the store to support the real time presence experience in a Sanity studio.

@simen simen requested review from rexxars and bjoerge November 2, 2017 17:33
this.handleRemoteChange()
}
// Update timestamp for this cache key
this.timestamps[key] = new Date()
Copy link
Member

Choose a reason for hiding this comment

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

I'd just use Date.now() instead of an actual Date since you won't actually be using any of the date functionality

performPurge = () => {
const now = new Date()
Object.keys(this.states).forEach(key => {
const age = now - this.timestamps[key].timestamp
Copy link
Member

Choose a reason for hiding this comment

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

Don't think timestamp is a standardized property, but as pointed out earlier, a Date.now() would skip the need for this

this.connection = connection
this.unsubscribeListener = this.connection.listen().subscribe(this.handleMessage).unsubscribe
this.myState = {}
this.states = {}
Copy link
Member

Choose a reason for hiding this comment

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

Perhaps use an ES6 Map for states and timestamps instead? It is more suitable for arbitrary keys, IIRC.

this.unsubscribeListener()
clearTimeout(this.resendReportTimer)
clearInterval(this.performPurgeTimer)
}
Copy link
Member

Choose a reason for hiding this comment

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

There are a couple more timers that should be closed here, I think? changeReportDebounceTimer for instance

@simen simen merged commit 19cb206 into next Nov 2, 2017
@bjoerge bjoerge mentioned this pull request Nov 7, 2017
@bjoerge bjoerge deleted the feature/presence-store branch November 8, 2017 15:37
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.

None yet

2 participants