adonisjs / lucid

AdonisJS SQL ORM. Supports PostgreSQL, MySQL, MSSQL, Redshift, SQLite and many more
https://lucid.adonisjs.com/
MIT License
1.03k stars 189 forks source link

serialize doesn't work with camelCase keys #896

Closed mastermunj closed 1 year ago

mastermunj commented 1 year ago

Explanation

While trying to pick fields for serialization, if a field name is specified as camelCase, it will not be included in the serialized output. However, when the field is specified as snake_case, it will be included in the serialized output.

Package version

lucid: 18.2.0

Node.js and npm version

Node: 18.7.0 Npm: 8.15.0

Sample Code (to reproduce the issue)

Please pull the sample repo and follow the steps. node ace serve --watch

Visit localhost:333/test-camel-case the output will have only id attribute, whereas id and created_at are expected.

Now, visit localhost:333/test-snake-case the output will have both id and created_at as expected.

BONUS (a sample repo to reproduce the issue)

https://github.com/mastermunj/adonis-serialize-test

DEBUG Information

Here are my findings so far, unless my understanding isn't correct.

Let's take an example of createdAt

https://github.com/adonisjs/lucid/blob/98644992cbd55cd1fdaa4b1fcf40d38540440083/src/Orm/BaseModel/index.ts#L1824-L1826

Here, column.serializeAs = created_at.

The method shouldSerializeField contains the following code. https://github.com/adonisjs/lucid/blob/98644992cbd55cd1fdaa4b1fcf40d38540440083/src/Orm/BaseModel/index.ts#L1135-L1139

pick is an array ['id', 'createdAt'] and the includes method receives created_at, which won't be found, thus the field doesn't be serialized.

thetutlage commented 1 year ago

This is intentional and by design. Also mentioned in the documentation.

Screenshot 2022-11-21 at 10 52 59 PM

The serialisation API most of times is used by the API clients (your frontend or the mobile app). The client only knows about the property names they see in the response and do not know about the model property names. Therefore, the serialisation works with that the client sees

mastermunj commented 1 year ago

Uh oh! I had a feeling that I most likely am missing reading the docs. Sorry for the bug, this should have rather gone to discussions.

My use case is to send only selected model attributes in the response, reducing the payload as well as keeping the remaining attributes hidden from the clients.

Will it be alright to allow both in the serialize? as in camelCase and snake_case? If yes, I can raise a PR for the same.

If not, will you be able to suggest an alternate solution?

thetutlage commented 1 year ago

Why not only select those fields in the first place?

mastermunj commented 1 year ago

That would have been the ideal thing to do, but many times the additional data needs to be fetched from the database for business processes, however, only a few attributes are required in the front end.

mastermunj commented 1 year ago

Will it be a breaking change to allow both camelCase and snake_case in the serialize? If not, I can attempt to raise a PR.

mastermunj commented 1 year ago

Hi @thetutlage, I have raised a PR that allows both camelCase (model property names) as well as snake_case (serialization property names).

Please help review.

mastermunj commented 1 year ago

Hi @thetutlage, I have added a few more comments on #902 to justify the need of allowing camelCase keys as well in the serialize. I'd appreciate it if you can spare some time to review and share your feedback.

mastermunj commented 1 year ago

Hi @thetutlage, I'll appreciate your feedback.

stale[bot] commented 1 year ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.