objection.js icon indicating copy to clipboard operation
objection.js copied to clipboard

snakeCaseMappers works differently for objects and FieldExpressions

Open Tapppi opened this issue 6 years ago • 5 comments

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.

Tapppi avatar Sep 24 '18 17:09 Tapppi

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

koskimas avatar Sep 25 '18 07:09 koskimas

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

I fixed this for us by:

  • copying the snakeCase from lib/utils/identifierMapping.js
  • creating a new snakeCaseMappers() factory that:
    • uses objection.snakeCaseMappers().parse() for parsing
    • uses the copied snakeCase for formatting, but only until the first :. Like mapLastPart, but until instead of from the separator

Tapppi avatar Sep 25 '18 13:09 Tapppi

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

koskimas avatar May 18 '19 07:05 koskimas

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

mindnektar avatar Nov 20 '19 13:11 mindnektar

@lehni I did a proposal for this issue 😃

cesumilo avatar Mar 22 '24 09:03 cesumilo