kiliman / remix-typedjson

This package is a replacement for superjson to use in your Remix app. It handles a subset of types that `superjson` supports, but is faster and smaller.
MIT License
435 stars 19 forks source link

Cannot read properties of undefined when properties with a `.` in the name clash. #22

Closed baker-travis closed 10 months ago

baker-travis commented 1 year ago

To start, I just want to say thanks for this project and the work that's gone into it. It's been really helpful to us.

Example reproduction here: https://github.com/baker-travis/typedjson-repro/blob/main/app/routes/index.tsx#L8

In that example, you can see that firstObject is a property on the root object, but so is firstObject.deeper. deeper is not a property on firstObject. Because of the way the meta is transferred with dot notation, when it tries to deserialize firstObject.deeper.anotherProperty, it assumes deeper is on the firstObject property, whereas the truth is that firstObject.deeper is its own property. The result is that useTypedLoaderData throws an error saying TypeError: Cannot read properties of undefined (reading 'anotherProperty').

The meta object that contains the info needed to add richer types back upon deserialization looks like this:

{
  "firstObject.deeper.anotherProperty": "date"
}

which doesn't give it enough info to actually know that firstObject.deeper needs to be accessed as a whole, and not as separate entities.

I'll leave my suggestion here. Something like this makes a lot of sense to me to transport that meta:

[
  {
    path: ["firstObject.deeper", "anotherProperty"],
    type: "date"
  }
]

It's more verbose, but depends on the actual keys instead of doing your own concatenation and parsing.

baker-travis commented 1 year ago

I opened up #23 with the suggestion I left, please let me know if you have any feedback.

kiliman commented 1 year ago

Ah, ok. A big assumption by typedjson is that . always separates objects.

Thanks for the PR. I'll review it.