ianstormtaylor / superstruct

A simple and composable way to validate data in JavaScript (and TypeScript).
https://docs.superstructjs.org
MIT License
6.95k stars 224 forks source link

[Docs] Document pass-by-reference pitfall for `defaulted` values #1236

Closed MaddyGuthridge closed 3 months ago

MaddyGuthridge commented 3 months ago

I just spent about half an hour trying to figure out why data was showing up in places it shouldn't. It turned out that my defaulted values were being passed by reference, meaning that any modifications to one instance would affect all current and future instances created from the same struct.

// BAD: array is passed by reference
const MyType = defaulted(array(string()), []);

const instanceA = create(undefined, MyType);

instanceA.push("hello");

const instanceB = create(undefined, MyType);
console.log(instanceB);
// [ "hello" ]

While this makes sense in hindsight, it was pretty painful to debug (especially since my case had a significantly more complex data structure).

As such, I think this project would be improved if this pitfall was documented. I've created a PR that adds this documentation.

Additionally, I noticed that the type signatures for defaulted allow any as the default value. Is this intentional? I imagine that stricter types would allow for significantly improved safety, helping users to avoid this issue (eg only accepting a plain value if the value is a type that cannot be modified).

MaddyGuthridge commented 3 months ago

Wait hang on a second my example doesn't work........ why on earth did changing it to a function fix this for my bug?

MaddyGuthridge commented 3 months ago

Ok looks like the issue is more complex than I thought, I'll close this issue and the related PR and open a new issue that describes the problem better.