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

Configurable camelizing type values #34

Closed
antonkomarev opened this issue Feb 14, 2018 · 6 comments
Closed

Configurable camelizing type values #34

antonkomarev opened this issue Feb 14, 2018 · 6 comments

Comments

@antonkomarev
Copy link
Contributor

antonkomarev commented Feb 14, 2018

As was discovered in PR #33 there is potential issue with camelizing type's values inside relationships data objects.

Despite camelizing option explicitly called: camelizeKeys: true it's affecting type values too.
It will be great to have separate configurable option to camelize type values.

Input:

const json = {
  data: [
    {
      type: 'post',
      relationships: {
        'rel1-to-camelize': {
          data: [
            {
              id: 4,
              type: 'type1-to-camelize',
            },
          ],
        },
      },
      id: 2620,
    },
  ],
};

Output:

const output = {
  post: {
    2620: {
      type: 'post',
      id: 2620,
      relationships: {
        rel1ToCamelize: {
          data: [
            {
              id: 4,
              type: 'type1ToCamelize',
            },
          ],
        },
      },
    },
  },
};

In my point of view this should work by this way:

result = normalize(json, {
    camelizeKeys: true,
    camelizeTypeValues: true,
});

And camelizeTypeValues should be in false state by default.

This is breaking change

@antonkomarev
Copy link
Contributor Author

After this implementation top-level type value should be affected by this option too.

@yury-dymov
Copy link
Owner

I don't mind but I would prefer that by defaul: camelizeTypeValues = camelizeKeys

@antonkomarev
Copy link
Contributor Author

antonkomarev commented Feb 14, 2018

I just don't see any case where you need to mutate values. It's clearly understood why it could be required for keys (for easier properties access), but for values it's not so obvious behavior. But maybe it's only for me.
Anyway if this will be configurable - it will be not a big problem. I think it's matter of taste.

@yury-dymov
Copy link
Owner

The thing is that I don't need this feature but I understand that others might need it.

I agree that mutating value is a weird behavior in a vacuum but maybe, just maybe some folks want to be consistent here and there.

@antonkomarev
Copy link
Contributor Author

antonkomarev commented Feb 15, 2018

I agree that consistency is important. And additionally there could be case when you need to compare type value with the resources stored in your local repository by their type key. And since they are both camelized by default this will be easier then.

@yury-dymov
Copy link
Owner

Thank you for the help :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants