[FEATURE ds-serialize-id] add serializeId to json-serializer #4620
Conversation
| @@ -1073,6 +1069,21 @@ var JSONSerializer = Serializer.extend({ | |||
| }, | |||
|
|
|||
| /** | |||
| SerializeId can be used to customize how 'Id' is seriaziled | |||
There was a problem hiding this comment.
SerializeId => serializeId
'Id' => id
seriaziled => serialized
Can we expand this with a use-case and example? I've seen a couple of issues where people want to cast the ID to a Number – that might be a good example.
| @@ -68,6 +68,20 @@ test("serialize includes id when includeId is true", function(assert) { | |||
| }); | |||
| }); | |||
|
|
|||
| test("serializeId", function(assert) { | |||
There was a problem hiding this comment.
Can we add another test and call serialize() with a customized serializeId() and assert that serializeId() is actually called from serialize()
There was a problem hiding this comment.
Since this adds a new method to the public API, this needs to go behind a feature flag. See the section in the CONTRIBUTING.md on how to do that. I would suggest ds-serialize-id for the feature name.
You can also take a look at #4110 for a sample PR which implements a feature.
Spasibo!
Romanior
force-pushed
the
add-serializeId
branch
from
07acf97
to
5a9759c
Compare
| var id = snapshot.id; | ||
| json[primaryKey] = parseInt(id, 10); | ||
| } | ||
| }); |
There was a problem hiding this comment.
Tiny nitpick but can you adapt the indentation of the code sample? Thanks!
| serializeId(snapshot, json, primaryKey) { | ||
| var id = snapshot.id; | ||
|
|
||
| if (id && primaryKey) { |
There was a problem hiding this comment.
I think the check for primaryKey can be removed. This would then be symmetric to the implementation when this feature is not enabled.
| var id = snapshot.id; | ||
| json[primaryKey] = parseInt(id, 10); | ||
| } | ||
| }); |
|
|
||
| /** | ||
| serializeId can be used to customize how id is serialized | ||
|
|
There was a problem hiding this comment.
I think it would be good to outline the default implementation. Something along the lines of:
By default the snapshots'
id(which is a String) will be set on the json hash viajson[primaryKey] = snapshot.id.
|
While you're at it, can you prefix the commit as |
Romanior
force-pushed
the
add-serializeId
branch
3 times, most recently
from
87cc598
to
f3119e5
Compare
|
Sorry, with |
Romanior
force-pushed
the
add-serializeId
branch
from
f3119e5
to
0a204bb
Compare
Romanior
changed the title
addresses #4477