1.0 API - Githubissues

scoutforpets / jsonapi-mapper

JSON API-Compliant Serialization for your Node ORM

The Unlicense

42 stars 24 forks source link

1.0 API #59

Closed jamesdixon closed 8 years ago

jamesdixon commented 8 years ago

Synopsis Discuss pros, cons and implementation of our own API that will prevent changes in the underlying serializer API from affecting the mapper.

See #57 for further discussion on this topic.

chamini2 commented 8 years ago

Well, some pros and cons I can come up with right now

PROS:

Not depending on another project's API, just it's functionality
Full documentation of the tool's capabilities (#30 was opened for not knowing the solution of using the serializer's API)
Achieving a high-level abstraction (scond paragraph)

CONS:

Maintaining a larger code base

EDIT: this could be used to propose changes to the names and documentation of the current options

jamesdixon commented 8 years ago

Based on a conversation w/ @chamini2, we'd like to finalize the API in the next beta and then release 1.0. We had specifically discussed our public API and how that maps to the serializer options.

Proposal

typeForModel - relates to the type returned in the payload. This would map to the typeForAttribute option in the serializer. typeForModel seems to make more sense than typeForAttribute. It would accept a function or an object containing relation/type mappings. By default, we would pass our own function that would pluralize the type.
keyForAttribute - accepts a function that manipulates the attribute keys. Note that this would not accept strings such as camelCase or dash-case as supported in the serializer. Reasoning is that if support was removed, we'd have to build this in to the mapper to prevent breakage. Best left to pure functions.

Thoughts on this?

cc: @ShadowManu @jcao02 @chamini2

chamini2 commented 8 years ago

Agreed with everything.

jcao02 commented 8 years ago

Agree

ShadowManu commented 8 years ago

Not going to disagree and be the party crasher ;) Only thing (also a question) would be to make clear on the documentation if the typeforModel affects the main model/collection (it doesn't, right?).

PS: since @chamini2 almost cried, I'd like to clarify I agree ;)

jamesdixon commented 8 years ago

@ShadowManu but you sound like you're itching to disagree! 😄

As for your question, typeForModel would affect the type of the model. Maybe I'm misunderstanding the quest?

chamini2 commented 8 years ago

I think he meant we should specify that typeForModel only specifies types for relations' models, not the main one.

jamesdixon commented 8 years ago

Gotcha. Thanks for clarifying.

jamesdixon commented 8 years ago

@chamini2 can we close this given your PR from a few days back?

chamini2 commented 8 years ago

Sure, I had forgotten