Vincit / objection.js

An SQL-friendly ORM for Node.js
https://vincit.github.io/objection.js
MIT License
7.24k stars 636 forks source link

snakeCaseMappers works differently for objects and FieldExpressions #1089

Open Tapppi opened 5 years ago

Tapppi commented 5 years ago

Using snakeCaseMappers, patching an object will not map inner keys, only the first level (columns), but using a FieldExpression will map the json field references.

class TestModel extends Model {
  ...
  static columnNameMappers = snakeCaseMappers();
  ...
}

// Query with array/object
TestModel
  .query()
  .patch({
    jsonColumn: [{
      innerKey: 1
    }]
  });

// Database output
{
  json_column: '[{"innerKey": 1}]' 
}

// Query with FieldExpression
TestModel
  .query()
  .patch({
    'jsonColumn:[0][innerKey]': 1
  });

// Database output
{
  json_column: '[{"inner_key": 1}]' 
}

EDIT: IMHO, this should work consistently across, and there could be an option for snakeCaseMappers | knexSnakeCaseMappers, something like deep|deepKeys|innerKeys. Obviously I would prefer it to default to false, and I'm guessing the usual expectation is around how the normal objects now work, but the default's not a big deal for me eitherway.

koskimas commented 5 years ago

Objection shouldn't touch the JSON fields at all. It's a bug that the inner keys actually do change in some case.

Tapppi commented 5 years ago

@koskimas agreed, the option would've been an additional feature based on the "hidden feature" ;)

I fixed this for us by:

koskimas commented 5 years ago

I needed to revert the fix for this. It caused more problems than it fixed.

mindnektar commented 4 years ago

any news on this? (thanks for your continued great work, by the way.)

cesumilo commented 6 months ago

@lehni I did a proposal for this issue 😃