Re-export cast utility functions from immer

[Methuselah96]

No description provided.

[codesandbox-ci]

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 49e1882:

Sandbox	Source
Vanilla	Configuration
Vanilla Typescript	Configuration
rsk-github-issues-example	Configuration
@examples-query-react/basic	Configuration
@examples-query-react/advanced	Configuration
@examples-action-listener/counter	Configuration

[netlify]

✅ Deploy Preview for redux-starter-kit-docs ready!

Name	Link
🔨 Latest commit	49e1882
🔍 Latest deploy log	https://app.netlify.com/sites/redux-starter-kit-docs/deploys/62730ae1983dad00084861a6
😎 Deploy Preview	https://deploy-preview-2308--redux-starter-kit-docs.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site settings.

[markerikson]

Yo! Any particular reason we need to re-export these specifically?

[Methuselah96]

They're necessary to make reducers type-safe in the situations mentioned here (most commonly if there's a readonly array in the state) and it would be nice to be able to use them without having to add a dependency on Immer.

[Methuselah96]

I can't think of a situation where I would use castImmutable in relation to RTK, so I would be fine with removing that one and just re-exporting castDraft. I just included it because they seemed related.

[phryneas]

Mark, Matt and I just discussed this a bit - we feel like this just doesn't happen often enough to add it as a re-export, and if it actually happens, adding immer as a dependency might actually be a better choice than us adding it for everyone.

[Methuselah96]

How much it's used is highly dependent on how often one uses read-only arrays in their state. For me this comes up quite frequently since I use read-only arrays for all the arrays that are in state, which means any usage of the part of state that includes arrays needs to use castDraft. I understand that most people aren't that strict about it and so it doesn't come up that often for those people, but I'm not sure I understand the downside of including it for those of us who like the strictness of making their state types fully read-only. I have no other reason to add immer as a dependency, so it would be really nice to not have to add it as a dependency just for this.

[Methuselah96]

I'd be happy to just cut it down to just re-exporting castDraft since I have no need for castImmutable, but it seems like a pretty straightforward re-export. Is there something particular about this re-export that you're concerned about long-term?

[Methuselah96]

@markerikson @phryneas Any more thoughts on what the concerns are for adding the export? Am I right in thinking that most people don't run into this because they're not using readonly arrays? I made a TS playground demonstrating the issue. If you make the array non-readonly you'll see that it makes it possible to mutate it when using the state somewhere else in the app. I run into the need to use castDraft very frequently and I see people on my team just using a non-readonly array just to get rid of the error, which leads to less type-safety. I think it would make sense to include castDraft to encourage readonly state types where possible, or just to throw a bone to those of us who care about it. :)

[markerikson]

It's a combination of a few things:

• Anything we add to the public API lives there forever
• We try to aim for the "80% common use case" scenario, and I'm not sure this qualifies
• I'm also trying to avoid turning RTK into "export * from Immer and Reselect", which is why we also don't re-export createStructuredSelector
• This is the first I've heard of readonly types being an issue, and thus far you're the only person who's brought this up :)

[Methuselah96]

Yeah, I get that, but this function is so simple that it seems like the danger of re-exporting it and it causing problems in the future it is extremely low.

Out of curiosity, do you use readonly arrays when typing arrays that live in state?

[phryneas]

It's not about the complexity of the function - but what if immer goes from 8 to 9 and removes it because they now do this on a type level with some new pattern? We'd need a major release to go to immer 9.

There are things that are central to RTK where we can't be picky, and then there is stuff like this where risks like that have to be weighed a bit more.

Also, it's not free - if the consumer's bundler doesn't tree-shake well, it adds another 10 bytes to the library - no matter if actually used or not.

[phryneas]

Out of curiosity, do you use readonly arrays when typing arrays that live in state?

Honestly, you're the first person I've seen doing that. I get that it could be a "better practice", but with immer automatically freezing things anyways, it's just never been any concern to me personally. ReadonlyArrays feel just very bulky to use for me because the ecosystem doesn't support that in the slightest.

[Methuselah96]

It's not about the complexity of the function - but what if immer goes from 8 to 9 and removes it because they now do this on a type level with some new pattern? We'd need a major release to go to immer 9.

There are things that are central to RTK where we can't be picky, and then there is stuff like this where risks like that have to be weighed a bit more.

Seems like you would need a major release in that case whether you exported castDraft or not and it would be easy to stub castDraft if necessary. I only mentioned the complexity since oftentimes a more complex function would be more likely to have breaking changes in its functionality over time, but the existence of the function seems to be fairly stable as well.

Honestly, you're the first person I've seen doing that. I get that it could be a "better practice", but with immer automatically freezing things anyways, it's just never been any concern to me personally. ReadonlyArrays feel just very bulky to use for me because the ecosystem doesn't support that in the slightest.

To each their own, I agree that the ecosystem does not support readonly arrays well.

Thanks for you responses, I appreciate your time.

[phryneas]

We have gone from immer 2 or so to currently immer 9 without the need for a bump - because the features we use & expose of it are stable "enough" (not 100% unfortunately, but nothing really breaking).

But the more we start pulling in external dependencies that we have no control of (afaik we control every other dependency), the flakier it gets. That's just why we have a bad feeling adding more - especially since this is essentially a type helper and those patterns do change over time.

[markerikson]

As it was, when Immer changed its built-in "enable the ES5 fallback by default" behavior to make it be a plugin instead, we had to switch to calling enableES5() ourselves in order to keep the same behavior and upgrade to the newer version of Immer in an RTK minor. We're finally going to get to drop that in RTK 2.0, but we did already deal with a dep changing here once already.

[Methuselah96]

Yeah, I understand that re-exporting any function from a dependency inherently increases risk and so its just a judgment call of whether the likelihood of the function changing is worth the gain in exporting the function. I see both the likelihood of the function changing and the impact that it would have if it did change to be very small, so it seems worth it to me, but it probably always seems worth it to the person who "really needs this one feature pls." :)

It is not hard for me to implement the function myself since RTK (thankfully) already exports the Draft type, but it's hard to encourage people to make their state fully read-only when the package maintainers (implicitly or explicitly) discourage it by not exporting the functions needed to make it fully read-only, but I guess that's just my burden for going against the grain. I understand that TypeScript is not trying to guarantee 100% type-safety and that in general the community don't use read-only types even when they are applicable, but I do like to make it as safe as possible when I can.

Again, I appreciate your time and responses.