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

When an intersection is going to produce an expression too complex error... #42772

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 38 additions & 3 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13728,7 +13728,7 @@ namespace ts {
function getIntersectionType(types: readonly Type[], aliasSymbol?: Symbol, aliasTypeArguments?: readonly Type[]): Type {
const typeMembershipMap: ESMap<string, Type> = new Map();
const includes = addTypesToIntersection(typeMembershipMap, 0, types);
const typeSet: Type[] = arrayFrom(typeMembershipMap.values());
let typeSet: Type[] = arrayFrom(typeMembershipMap.values());
// An intersection type is considered empty if it contains
// the type never, or
// more than one unit type or,
Expand Down Expand Up @@ -13789,6 +13789,41 @@ namespace ts {
result = getUnionType([getIntersectionType(typeSet), nullType], UnionReduction.Literal, aliasSymbol, aliasTypeArguments);
}
else {
let runningResult: Type | undefined;
const originalSet = typeSet;
if (typeSet.length > 2 && getCrossProductUnionSize(typeSet) >= 100000 && every(typeSet, t => !!(t.flags & TypeFlags.Union) || !!(t.flags & TypeFlags.Primitive))) {
// This type set is going to trigger an "expression too complex" error below. Rather than resort to that, as a last, best effort, when
// the intersection looks like (A | B | C) & (D | E | F) & (G | H | I) - in the general case, this can result in a massive resulting
// union, hence the check on the cross product size below, _however_ in some cases we can simplify the resulting type massively
// - if we can recognize that upfront, we can still allow the type to form without creating innumerable intermediate types.
// Specifically, in cases where almost all combinations are known to reduce to `never` (so the result is essentially sparse)
// and we can recognize that quickly, we can use a simplified result without checking the worst-case size.
// So we start with the assumption that the result _is_ sparse when the input looks like the above, and we assume the result
// will take the form (A & D & G) | (B & E & H) | (C & F & I). To validate this, we reduce left, first combining
// (A | B | C) & (D | E | F); if that combines into `(A & D) | (B & E) | (C & F)` like we want, which we make 9 intermediate
// types to check, we can then combine the reduced `(A & D) | (B & E) | (C & F)` with (G | H | I), which again takes 9 intermediate types
// to check, finally producing `(A & D & G) | (B & E & H) | (C & F & I)`. This required 18 intermediate types, while the standard method
// of expanding (A | B | C) & (D | E | F) & (G | H | I) would produce 27 types and then perform reduction on the result.
// By going elemnt-wise, and bailing if the result fails to reduce, we can allow these sparse expansions without doing undue work.
runningResult = typeSet[0];
for (let i = 1; i < typeSet.length; i++) {
// for intersection reduction, here we're considering `undefined & (A | B)` as `never`. (ie, we're disallowing branded primitives)
// This is relevant for, eg, when looking at `(HTMLElement | null) & (SVGElement | null) & ... & undefined` where _usually_
// we'd allow for tons of garbage intermediate types like `null & SVGElement` to exist; but nobody ever really actually _wants_
// that, IMO. Those types can still exist in the type system; just... not when working with unions and intersections with massive
// cross-product growth potential.
Copy link
Member Author

@weswigham weswigham Feb 12, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RE: This comment and associated logic. If we did this in all scenarios, it would be a breaking change; however since we only do it when we would have previously issued a Expression produces a union type that is too complex to represent error, it is not. I think we've had a desire to try to eliminate structurally branded primitives from the type system for awhile, and I think this is just one small corner where we can actually remove them and be pretty safe, without worrying about either breaking people or needing to provide an alternative.

runningResult = typeSet[i].flags & TypeFlags.Primitive && everyType(runningResult, t => !!(t.flags & TypeFlags.Object)) ? neverType : getReducedType(intersectTypes(runningResult, typeSet[i]));
if (i === typeSet.length - 1 || isTypeAny(runningResult) || runningResult.flags & TypeFlags.Never) {
return runningResult;
}
if (!(runningResult.flags & TypeFlags.Union) || (runningResult as UnionType).types.length > typeSet.length) {
// save work done by the accumulated result thus far, even if we're bailing on the heuristic
// (it may have saved us enough work already that we're willing to work with the type now)
typeSet = typeSet.slice(i + 1);
break;
}
}
}
// We are attempting to construct a type of the form X & (A | B) & (C | D). Transform this into a type of
// the form X & A & C | X & A & D | X & B & C | X & B & D. If the estimated size of the resulting union type
// exceeds 100000 constituents, report an error.
Expand All @@ -13798,8 +13833,8 @@ namespace ts {
const constituents = getCrossProductIntersections(typeSet);
// We attach a denormalized origin type when at least one constituent of the cross-product union is an
// intersection (i.e. when the intersection didn't just reduce one or more unions to smaller unions).
const origin = some(constituents, t => !!(t.flags & TypeFlags.Intersection)) ? createOriginUnionOrIntersectionType(TypeFlags.Intersection, typeSet) : undefined;
result = getUnionType(constituents, UnionReduction.Literal, aliasSymbol, aliasTypeArguments, origin);
const origin = some(constituents, t => !!(t.flags & TypeFlags.Intersection)) ? createOriginUnionOrIntersectionType(TypeFlags.Intersection, originalSet) : undefined;
result = runningResult ? getIntersectionType([runningResult, getUnionType(constituents, UnionReduction.Literal)], aliasSymbol, aliasTypeArguments) : getUnionType(constituents, UnionReduction.Literal, aliasSymbol, aliasTypeArguments, origin);
}
}
else {
Expand Down
89 changes: 89 additions & 0 deletions tests/baselines/reference/jsxLargeRefUnion.errors.txt

Large diffs are not rendered by default.

119 changes: 119 additions & 0 deletions tests/baselines/reference/jsxLargeRefUnion.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,119 @@
//// [jsxLargeRefUnion.tsx]
/// <reference path="/.lib/react16.d.ts" />

import * as React from "react";

const animated: {
[Tag in keyof JSX.IntrinsicElements]: React.ForwardRefExoticComponent<
React.ComponentPropsWithRef<Tag>
>
} = {} as any;

function makeAnimated<T extends React.ElementType<any>>(
comp: T
): React.ForwardRefExoticComponent<React.ComponentPropsWithRef<T>> {
return null as any; // not important
}

export interface UpgradedProps {
show: boolean;
}

export function test<P>(
component: React.ComponentType<P> | keyof React.ReactHTML
): React.ComponentType<P & UpgradedProps> {
// changing to `const Comp: any` un-hangs tsserver
const Comp =
typeof component === "string"
? animated[component]
: makeAnimated(component);

return React.forwardRef<any, P & UpgradedProps>((props, ref) => {
const { show, ...ownProps } = props;
return show ? <Comp {...ownProps} ref={ref} /> : null; // ref as currently defined is expression-too-complex
});
}

type FixedRef<T> = string | null | React.RefObject<T> | { bivarianceHack(instance: T | null): any }["bivarianceHack"] & {current?: undefined};
declare module "react" {
interface DOMElement<P extends HTMLAttributes<T> | SVGAttributes<T>, T extends Element> extends ReactElement<P> {
customRef: FixedRef<T>;
}
}
interface ForwardCustomRefRenderFunction<T, P = {}> {
(props: React.PropsWithChildren<P>, ref: FixedRef<T>): React.ReactElement<any> | null;
displayName?: string;
defaultProps?: never;
propTypes?: never;
}
declare function forwardCustomRef<T, P = {}>(Component: ForwardCustomRefRenderFunction<T, P>): React.ComponentType<P & React.ClassAttributes<T>>;

export function test2<P>(
component: React.ComponentType<P> | keyof React.ReactHTML
): React.ComponentType<P & UpgradedProps> {
// changing to `const Comp: any` un-hangs tsserver
const Comp =
typeof component === "string"
? animated[component]
: makeAnimated(component);

return forwardCustomRef<any, P & UpgradedProps>((props, ref) => {
const { show, ...ownProps } = props;
return show ? <Comp {...ownProps} customRef={ref} /> : null; // with the additional `current?: undefined` member on the signature, it now can resolve
});
}

//// [jsxLargeRefUnion.js]
"use strict";
/// <reference path="react16.d.ts" />
var __assign = (this && this.__assign) || function () {
__assign = Object.assign || function(t) {
for (var s, i = 1, n = arguments.length; i < n; i++) {
s = arguments[i];
for (var p in s) if (Object.prototype.hasOwnProperty.call(s, p))
t[p] = s[p];
}
return t;
};
return __assign.apply(this, arguments);
};
var __rest = (this && this.__rest) || function (s, e) {
var t = {};
for (var p in s) if (Object.prototype.hasOwnProperty.call(s, p) && e.indexOf(p) < 0)
t[p] = s[p];
if (s != null && typeof Object.getOwnPropertySymbols === "function")
for (var i = 0, p = Object.getOwnPropertySymbols(s); i < p.length; i++) {
if (e.indexOf(p[i]) < 0 && Object.prototype.propertyIsEnumerable.call(s, p[i]))
t[p[i]] = s[p[i]];
}
return t;
};
exports.__esModule = true;
exports.test2 = exports.test = void 0;
var React = require("react");
var animated = {};
function makeAnimated(comp) {
return null; // not important
}
function test(component) {
// changing to `const Comp: any` un-hangs tsserver
var Comp = typeof component === "string"
? animated[component]
: makeAnimated(component);
return React.forwardRef(function (props, ref) {
var show = props.show, ownProps = __rest(props, ["show"]);
return show ? React.createElement(Comp, __assign({}, ownProps, { ref: ref })) : null; // ref as currently defined is expression-too-complex
});
}
exports.test = test;
function test2(component) {
// changing to `const Comp: any` un-hangs tsserver
var Comp = typeof component === "string"
? animated[component]
: makeAnimated(component);
return forwardCustomRef(function (props, ref) {
var show = props.show, ownProps = __rest(props, ["show"]);
return show ? React.createElement(Comp, __assign({}, ownProps, { customRef: ref })) : null; // with the additional `current?: undefined` member on the signature, it now can resolve
});
}
exports.test2 = test2;
Loading