Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DeepReadonly causes too complex errors for simple case #194

Open
cefn opened this issue Apr 1, 2021 · 3 comments
Open

DeepReadonly causes too complex errors for simple case #194

cefn opened this issue Apr 1, 2021 · 3 comments
Labels
enhancement New feature or request

Comments

@cefn
Copy link

cefn commented Apr 1, 2021

I produced a minimal repro as a typescript playground here

I was aiming to migrate to using DeepReadonly instead of my home-rolled recursive Immutable see here, but I found that switching to DeepReadonly prevented my apps from compiling since there's something creating an explosion of complexity compared to my original Immutable implementation.

The worked example is a fairly self-explanatory React style component, which consumes a spriteSheet and a spriteName in order to render a named tile from the sheet. The names are drawn from a type-safe union of strings.

It's a real-world case but I removed all the unnecessary cruft you don't need to see.

The playground shows the errored case (where DeepReadonly is used) and also shows how to comment out the DeepReadonly implementation in favour of my simpler Immutable which eliminates the complexity errors and compiles correctly.

It was confusing at first since the reported error is as follows...

Type 'DeepReadonly<{ [N in TileName]: [number, number]; }>[TileName]' must have a 'Symbol.iterator' method that returns an iterator.

However, that's actually masking a complexity error which reads...

Expression produces a union type that is too complex to represent.

...and I think the fact this bails with an unexpanded type causes the other spurious error.

If anyone is curious regards the 'real world' case, this issue arose as a failing CI for a PR against an otherwise clean-compiling repo which attempted the change to DeepReadonly

@quezak
Copy link
Collaborator

quezak commented Apr 1, 2021

Our DeepReadonly was once simpler too, kinda similar to your Immutable 😃

Unfortunately it grew to the current complexity to produce fully correct types for sets, weaksets, etc -- which is necessary in many different edge cases if you use those types. We'll gladly welcome any improvement PRs that still works in all these edge cases.

@cefn
Copy link
Author

cefn commented Apr 1, 2021

I think it may have arrived at the point that it can't handle even quite simple cases given the hard limits imposed on complexity in recent Typescript versions. I don't see the element of my own type system that makes it complex - perhaps I missed an important constraint?

Is there a value in splitting out a DeepJSON or similar - only handles immutifying the limited set of types which are valid in JSON?

That leaves DeepReadonly for those using complex objects like Map,Regexp but with a small enough grammar that typescript can swallow it.

For reference, at least for my case, reducing the definition like this makes the type complexity small enough to compile. Adding pretty much anything back (e.g. Date, RegExp) makes it die.

@C0ZEN
Copy link

C0ZEN commented Apr 25, 2021

The official related issue.
Maybe a workaround here if someone want to take a look and provide a PR to enhance it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants