-
Notifications
You must be signed in to change notification settings - Fork 5
🤖 User test baselines have changed for fix-leaked-independent-type-params #123
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
base: user-baseline-updates
Are you sure you want to change the base?
🤖 User test baselines have changed for fix-leaked-independent-type-params #123
Conversation
node_modules/ts-toolbelt/out/index.d.ts(3527,92): error TS2321: Excessive stack depth comparing types 'MinusNegative<N1, N2, ?>' and 'MinusNegative<N1, N2, ?>'. | ||
node_modules/ts-toolbelt/out/index.d.ts(3527,92): error TS2321: Excessive stack depth comparing types 'MinusPositive<?, N2, IMap>' and 'MinusPositive<?, N2, IMap>'. | ||
node_modules/ts-toolbelt/out/index.d.ts(3527,92): error TS2321: Excessive stack depth comparing types 'MinusPositive<N1, ?, IMap>' and 'MinusPositive<N1, ?, IMap>'. | ||
node_modules/ts-toolbelt/out/index.d.ts(3527,92): error TS2321: Excessive stack depth comparing types 'MinusPositive<N1, N2, ?>' and 'MinusPositive<N1, N2, ?>'. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@millsp you wouldn't happen to know offhand if this can be worked around trivially in ts-toolbelt
or not, would you? I can look into a workaround for my change to avoid triggering these, but this probably indicates that if you caused these types to get their variance measured before, they'd issue this error.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think (I assume) that it comes from:
and
which are mistakes. Just for testing, if you can put number
instead of N2
on both lines, then (i believe) it should work. Let me know, because there might be other optimizations that I had to work on :) In any case, TypeScript has to move forward, and I will adapt for future releases.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, you can definitely trigger these errors in currently shipping typescript (or at least the current nightly) - just force variance calculation of the type by using it in a comparison of some kind (doesn't even have to make much sense):
type Box<X> = {f: MinusNegative<Cast<X, Iteration>, Cast<X, Iteration>, Map>};
const x = null as any as Box<number>;
const y: Box<12> = x;
y
will trigger all these Excessive stack depth
errors. :(
So if you can fix it, you probably should - it might already be causing headaches for your consumers.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changing those two lines to number
as you suggested doesn't seem to fix the issue, however.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will look into it
8e608d7
to
647a50b
Compare
acde8c3
to
b82007b
Compare
48d4889
to
78bf52e
Compare
This test run was triggerd by a request on microsoft#42211
Please review the diff and merge if no changes are unexpected.
You can view the build log here.
cc @weswigham