Properties with dots in their name are treated as paths instead of as literals

[VanTanev]

Given the following code:

export const db = factory({
  user: {
    'employee.id': primaryKey(() => 'abc-123'),
  }
})

db.user.create({ 'employee.id': 'some-id' })

We get this error:

Uncaught (in promise) OperationError: Failed to create a "user" entity: expected the primary key "employee.id" to have a value, but got: undefined
    at new OperationError (OperationError.js:28)
    at Object.create (factory.js:66)

This is because createModel uses lodash/set, which converts dot strings to deep properties (eg, among others): https://github.com/mswjs/data/blob/c4a5208301660f4abcfa9996ce301c34f5047aa0/src/model/createModel.ts#L58-L65

The resulting model object is:

  const user: {
    employee: {
      id: <primaryKey>
    }
  }
  // instead of 
  const user: {
    'employee.id': <primaryKey>
  }

I think that lodash/set is a poor fit, because properties with dots in them are completely valid. Is there any reason it was used other than convenience?

[kettanaito]

Hey, @VanTanev. Thanks for reporting this.

lodash.set was used to support nested objects (#113). Internally, we keep a flat map of deep object keys and use set to properly construct the end entities based on the model.

That's a valid concern that model properties that contained dots initially will be treated as nested properties by lodash. We need to account for such properties.

I suggest we preserve the original model properties by wrapping them in [] before passing to .set.

set(target, '["a.b.c"]', value)
// { "a.b.c": value }

This change will affect the following functions:

https://github.com/mswjs/data/blob/c4a5208301660f4abcfa9996ce301c34f5047aa0/src/model/createModel.ts#L59-L63

https://github.com/mswjs/data/blob/c4a5208301660f4abcfa9996ce301c34f5047aa0/src/model/createModel.ts#L80

https://github.com/mswjs/data/blob/c4a5208301660f4abcfa9996ce301c34f5047aa0/src/model/createModel.ts#L85

We need also be careful about definePropertyAtPath:

https://github.com/mswjs/data/blob/c4a5208301660f4abcfa9996ce301c34f5047aa0/src/utils/definePropertyAtPath.ts#L24

@VanTanev, would you be interested in opening a pull request with the fix? Adding a simple integration test that fails at the moment would be a great start. Let me know if this interests you, I'd be happy to help you during the code review.

[kettanaito]

It's worth analyzing if nested objects are needed in createModel to begin with. Perhaps we can use plain property assignment there and utilize deep objects when creating entities.

[VanTanev]

I'll try to create a PR. Might be a few days until I have the time.

On Tue, Oct 26, 2021, 17:52 Artem Zakharchenko @.***> wrote:

It's worth analyzing if nested objects are needed in createModel to begin with. Perhaps we can use plain property assignment there and utilize deep objects when creating entities.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/mswjs/data/issues/149#issuecomment-952020785, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABROHYPEQKBVFWIKONNYFTUI3FCJANCNFSM5GXJDM7Q .

[VanTanev]

Hey @kettanaito, I have already implemented this in #152. Is there anything you need me to improve?

[kettanaito]

@VanTanev, thank you for your work on that! I've seen the pull request, will review it as soon as I've got a spare minute. Looking forward to having this issue resolved.