search

scoutforpets / jsonapi-mapper

JSON API-Compliant Serialization for your Node ORM
The Unlicense
42 stars 24 forks source link

Changes on attribute fields from kebab case to snake case #80

Closed npatmaja closed 8 years ago

npatmaja commented 8 years ago

Hi, I just updated to beta 7 and I notice that the attributes fields changed from kebab case to snake case, which broke my project. For example my table column is transaction_status and in beta 6 and prior it is mapped to transaction-status but in beta 7 it is mapped to transaction_status. Is this behavior intentional?

chamini2 commented 8 years ago

The kebab case was a default of the jsonapi-serializer, we now by default don't change the casing, since there is no prefered casing expressed by the JSONAPI standard.

If you want this to be your default, you could do:

const ChangeCase = require('change-case');
const Mapper = require('jsonapi-mapper');

const URL = 'http://localhost:3000';

// Mapper instance
const mapper = new Mapper.Bookshelf(URL, { keyForAttribute: ChangeCase.paramCase });

@jamesdixon, what do you think of this? Should kebab / param case be the default or doing nothing is the right thing here?

jamesdixon commented 8 years ago

@chamini2 the JSON API spec doesn't specify one way or the other, but all of their examples use kebab-case. It's also the serializer default. My suggestion is that we do nothing here and as you've shown, allow users to make their own decisions in this regard.

jamesdixon commented 8 years ago

@npatmaja I just noticed that you said that your fields changed from kebab-case to snake_case in beta7. Just wanted to double check that your wording was correct? If that truly is the case, there might be an issue, but as @chamini2 mentioned, we don't tamper with the casing -- we leave that up to the user to specify.

npatmaja commented 8 years ago

@jamesdixon yes, I have tests that asserts the resulting object to contain correct attributes, which uses kebab-case. However, when I updated to beta 7 today, my tests failed and it turned out the attributes were in snake_case, but when I downgraded it to beta 6 again, the tests passed, without changing any code (except package.json)

As @chamini2 mentioned, an option to select a case will work if this behaviour is intentional.

jamesdixon commented 8 years ago

@npatmaja thanks for the follow up.

@chamini2 not sure if that was an intentional change or not?

chamini2 commented 8 years ago

It wasn't intentional, to be honest, but it makes sense to let it as a user's decision. Maybe we could use that new Mapper.Bookshelf options object as default behaviours to be passed by the user, and then they can overwrite a default behaviour by passing the same option in the .map options object?

jamesdixon commented 8 years ago

@chamini2 I believe that makes a lot of sense.

npatmaja commented 8 years ago

@jamesdixon @chamini2 :+1: I think we can close this issue now