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

styled-components hits instantiation limit after re-aliasing support (#42284) #42320

Closed
sandersn opened this issue Jan 13, 2021 · 3 comments · Fixed by #42430
Closed

styled-components hits instantiation limit after re-aliasing support (#42284) #42320

sandersn opened this issue Jan 13, 2021 · 3 comments · Fixed by #42430
Assignees
Labels
Bug A bug in TypeScript Fix Available A PR has been opened for this issue

Comments

@sandersn
Copy link
Member

export type StyledComponentPropsWithRef<
    C extends keyof JSX.IntrinsicElements | React.ComponentType<any>
> = C extends AnyStyledComponent
    ? React.ComponentPropsWithRef<StyledComponentInnerComponent<C>> // error is here
    : React.ComponentPropsWithRef<C>;

This is very likely because there are more type identities than before. I'm not sure whether the fix will have to be here or on the styled-components types.

@ahejlsberg
Copy link
Member

I took a look at this. There's definitely a latent circularity in the StyledComponentInnerComponent<C> type. When evaluating the C extends StyledComponent<infer I, any, any, any> condition, if C isn't exactly a StyledComponent<...> instantiation, we end up structurally inferring from the members of C to StyledComponent<I, any, any, any>. If C for example is instantiated to StyledComponentBase<...> we eventually end up inferring between the withComponent properties. But the return type of withComponent is StyledComponent<StyledComponentInnerComponent<WithC>, ...> and that takes us back to evaluating StyledComponentInnerComponent<C> again. And around the circle we go until the instantiation limiter kicks in.

I'm not sure why this doesn't trigger in 4.1, but it is undoubtedly because we are worse at preserving type aliases and therefore less precise in inference.

I can make the issue go away by removing the first overload of withComponent in StyledComponentBase. No tests seem to depend on it, but I don't know if there is code in the wild that does.

@alloy
Copy link
Member

alloy commented Jan 20, 2021

Very off-topic, but I would be very interested in a “look over your shoulder” session where you debug such an issue and learn the tricks/tooling you apply/use to unwrap the issue. I tend to find solutions for most issues I’m faced with if I keep at it long enough, but my blunt approaches feel very unscientific and leave me wondering how the ‘pros’ solve them.

@weswigham
Copy link
Member

weswigham commented Jan 21, 2021

I have some insight on why this regressed: Because of a scenario where we fail to preserve an alias where we used to.

To start, this wasn't an error before because it hit up our alias-comparison machinery in inference, rather than a structural inference, which, in addition to performing inferences quickly, prevents us from needing to eagerly resolving the structure of the object. As for why we preserved the alias before... It's a long story. We have a type like

type AnyStyledComponent = StyledComponent<any, any, any, any> | StyledComponent<any, any, any>;

where StyledComponent is an intersection of a few things. We then check against it in a conditional, eg, C extends AnyStyledComponent ? React.ComponentPropsWithRef<StyledComponentInnerComponent<C>> : React.ComponentPropsWithRef<C> - this makes C in the true branch be a substitution mapping to AnyStyledComponent & C. Our new alias preservation machinery gets that origin recorded. When we distribute over the union that AnyStyledComponent & C really is to fetch the constraint, we look at StyledComponent<any, any, any, any> & C and StyledComponent<any, any, any> & C individually. When we in turn get the constraints of those types, the C effectively drops away, and we're left with types which should be equivalent to StyledComponent<any, any, any, any> and StyledComponent<any, any, any>, but instead all the alias and origin work has been stripped away and we get the raw (string & StyledComponentBase<any, any, any, any> & NonReactStatics<any, {}>) and (string & StyledComponentBase<any, any, any, never> & NonReactStatics<any, {}>) types. Prior to the alias change, since the intersections were interned, even if we failed to traffic the alias symbols for a bit internally, so long as the type already existed (as it does here), if manipulations came back to the same type, it'd still have the alias, however our new alias logic punishes us for not trafficing alias information perfectly by stripping alias data irreversibly.

So the question then becomes "at what point during checking did we lose alias data"? To which the answer is, "when we called getBaseConstraint on the AnyStyledComponent type". computeBaseConstraint's union and intersection logic makes no effort to preserve alias data, even when the types flow through it unchanged - so you always get a new union/intersection out, without any alias data.

So the fix for this on our side is actually quite small, and I now have a PR up at #42430.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A bug in TypeScript Fix Available A PR has been opened for this issue
Projects
None yet
5 participants