-
Notifications
You must be signed in to change notification settings - Fork 86
For $ExpectType assertions, alphabetically sort unions and intersections #61
Conversation
There's going to be an immediate limitation with type literals containing unions (i.e. |
I didn't see a lot, so added code to just skip anything with an object literal in it for now. |
src/rules/expectRule.ts
Outdated
@@ -308,6 +309,27 @@ function getExpectTypeFailures( | |||
} | |||
} | |||
|
|||
function fixupType(s: string): string { | |||
// Don't mess up `{ a: number | string }` into `string } | { a: number` | |||
if (s.includes("{")) { |
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.
or <
, (
, [
If we don't expect this to work perfectly, why not try to see if the current ones are identical, then try the adjustment as a fallback mechanism? Does that sound reasonable? |
@@ -298,7 +298,12 @@ function getExpectTypeFailures( | |||
|
|||
const actual = checker.typeToString(type, /*enclosingDeclaration*/ undefined, ts.TypeFormatFlags.NoTruncation); | |||
if (actual !== expected) { | |||
unmetExpectations.push({ node, expected, actual }); | |||
console.log("!!!", actual); |
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.
Debug logs probably need to be cleaned up in this method/if-block?
Not a perfect solution to microsoft/TypeScript#17944 as it operates on strings and thus can't delve into types easily, but should be enough to fix our test failures for now.
When this is merged I'll have to update all the tests on DT to align with this at the same time this is published.