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

[DATA] RecordData Operations #458

Open
wants to merge 7 commits into
base: master
from

Conversation

Projects
None yet
4 participants
@runspired
Copy link
Contributor

runspired commented Mar 6, 2019

Rendered

runspired added some commits Mar 6, 2019

@rwjblue
Copy link
Member

rwjblue left a comment

Seems like a number of things are still missing from this WIP:

  • explicit list of all operations
  • explicit list of all methods to be deprecated on record data
  • "how do we teach this" section / etc
Show resolved Hide resolved text/0000-ember-data-record-data-operations.md Outdated
Show resolved Hide resolved text/0000-ember-data-record-data-operations.md Outdated
Show resolved Hide resolved text/0000-ember-data-record-data-operations.md Outdated
property?: string;
// the new value for the property on the record (if any)
// if this key is present, this value can be anything but `undefined`
value?: Identifier | null | array | string | object | number | boolean;

This comment has been minimized.

@rwjblue

rwjblue Mar 6, 2019

Member

Seems like this is just unknown, right?

This comment has been minimized.

@runspired

runspired Mar 9, 2019

Author Contributor

@rwjblue I don't think so because it's anything but undefined and unknown would imply undefined was possible, yes?

This comment has been minimized.

@chriskrycho

chriskrycho Mar 13, 2019

@runspired unfortunately, ? on the declaration it means it's still allowed to pass undefined – see this playground for an example.

Also, note that array here makes sense descriptively but isn't valid TS, for what that's worth.

This comment has been minimized.

@runspired

runspired Mar 13, 2019

Author Contributor

:'(

Show resolved Hide resolved text/0000-ember-data-record-data-operations.md Outdated

@runspired runspired changed the title [WIP] Record Data Operations [WIP DATA] RecordData Operations Mar 7, 2019

rwjblue and others added some commits Mar 9, 2019

Update text/0000-ember-data-record-data-operations.md
Co-Authored-By: runspired <runspired@users.noreply.github.com>

@runspired runspired changed the title [WIP DATA] RecordData Operations [DATA] RecordData Operations Mar 12, 2019

@dgeb

This comment has been minimized.

Copy link
Member

dgeb commented Mar 13, 2019

Let's consider using Orbit's operations, which are very close in signature to the proposal here, in order to maximize compatibility: https://github.com/orbitjs/orbit/blob/master/packages/%40orbit/data/src/operation.ts#L4-L106

@dgeb

This comment has been minimized.

Copy link
Member

dgeb commented Mar 13, 2019

This proposal seems incomplete without operations that create, update, and remove records as a whole.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.