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

Record Data State #463

Merged
merged 1 commit into from
May 22, 2019
Merged

Record Data State #463

merged 1 commit into from
May 22, 2019

Conversation

igorT
Copy link
Member

@igorT igorT commented Mar 13, 2019

@igorT igorT added the T-ember-data RFCs that impact the ember-data library label Mar 13, 2019
@mikkopaderes
Copy link

Are there any plans for an isUnloaded state? Current use-case is for an adapter that supports realtime changes (e.g. Firebase). Whenever someone loses permission or a record was deleted some adapters might want to unload those records in realtime. However, I don't think there's anything that exists to indicate this state.

Another related property may be unloadReason where it'll contain the reason why the record is in isUnloaded: true state.

@runspired
Copy link
Contributor

runspired commented Mar 21, 2019

@mikkopaderes the short answer is that there is not. We did explore whether recordData should consider unload as an operation on RecordState, and the clear answer was "no".

A TL;DR of what to do instead is check if the RecordData cache has an entry for a given identifier

If we start with a bit of context on what unloadRecord is meant to achieve and how that fits in both the pre-record-data and post-record-data worlds helps to understand the semantics here.

In pre-record-data ember-data, InternalModel fulfilled many roles including:

  • being the identifier for a resource that may or may not have been loaded
    Read the Identifiers RFC for more context and insight on how we are replacing and improving that role.

  • being the cache for attributes/relationships for a resource (now handled by RecordData)

  • being the cache for loading & network state: see Request State RFC

  • being the cache for RecordState (this RFC)

  • being the cache for Error State (but not errors themselves which were stashed on record instances)
    Read the RecordData Errors RFC

An unfortunate design within InternalModel was that loading|network|error|record state were mixed into a single state machine. You can see (from the RFCs above) that this is something we are working to rectify.

With the InternalModel design, unloadRecord played a poorly fit role of needing to change the state of the InternalModel from wherever it was back into empty, needing to notify any relate records that it would need to be loaded again if re-accessed, and of actually emptying the cache.

In the RecordData design, unloadedRecords are simply absent from the cache, with knowledge of their existence maintained if needed via an Identifier. By separating the identity from the data, we can seamlessly release the data, and use the absence of it to know we need to retrieve it when asked.

Copy link
Member

@rwjblue rwjblue left a comment

Choose a reason for hiding this comment

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

One minor inline comment, but I’d also like to confirm that this RFC has to land after the identifier RFC #403, right?


isDeleted(identifier: RecordIdentifier): boolean

isDeletionPersisted(identifier: RecordIdentifier): boolean
Copy link
Member

Choose a reason for hiding this comment

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

Bizarre spacing here, maybe tabs vs spaces or something?

getAttr(identifier: RecordIdentifier, key: string): any;
getRelationship(identifier: RecordIdentifier): JsonApiRelationship;

performMutation(operation: Operation): void
Copy link
Member Author

Choose a reason for hiding this comment

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

Remove from RFC before merging

@igorT igorT merged commit 33584d0 into emberjs:master May 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Final Comment Period T-ember-data RFCs that impact the ember-data library
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants