Uh oh!
There was an error while loading. Please reload this page.
Reuse input type nodes when serializing signature parameter and return types - #37444
Conversation
…scope with current node, like property types, only reuse nodes if symbols referened are acessible, reuse nodes for property signatures, too
…ipt into js-decls-unique-symbol
| function bar(x: {new(): Foo;}){} | ||
| >bar : (x: new () => Foo) => void | ||
| >bar : (x: { new (): Foo;}) => void |
There was a problem hiding this comment.
I see how the signature annotation changed but shouldnt it be { new (): Foo; } or {new(): Foo;} instead ? Why is there extra space before new
There was a problem hiding this comment.
I'm pretty sure, it's because our default formatting for object literals which don't have any emit flags set is newline-separated with appropriate indentation. I could set some emit flags to try to make the objects single-line; but I imagine that default is actually better when we're not using the single line string writer that we use for .types baselines and error messages. (eg, quickinfo, declarations)
There was a problem hiding this comment.
Hmm even if its our default one wit wouldnt print it shouldnt start with these extra spaces no?
There was a problem hiding this comment.
If it actually emits as
{new(): Foo;}and the printer is simply eliding newlines (it is), it would be.
| export function f4(arg1: | ||
| >f4 : (arg1: {}) => void | ||
| >f4 : (arg1: { [number]: C1;}) => void |
| @@ -30,9 +30,9 @@ verify.quickInfoIs("var t: FooHandler"); | |||
| goTo.marker("2"); | |||
| verify.quickInfoIs("var t2: FooHandler2"); | |||
| goTo.marker("3"); | |||
| verify.quickInfoIs("type FooHandler2 = (eventName?: string, eventName2?: string) => any", "- What, another one?"); | |||
| verify.quickInfoIs("type FooHandler2 = (eventName?: string | undefined, eventName2?: string) => any", "- What, another one?"); | |||
There was a problem hiding this comment.
This seems like unwanted result of additional undefined given the parameter was written as {string=}
There was a problem hiding this comment.
{string=} can appear in not-ending positions, which still add | undefined to the parameter type, but don't add optionality to the parameter, so the existing node walker conservatively adds | undefined to the type nodes any time it sees the jsdoc optional type marker, since it's not semantically tracking if the = is actually implying a ? or not. In the real world, the extra | undefined won't do anything if the ? is present, so including it won't break anything, at least.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Wesley Wigham (weswigham)
commented
Apr 1, 2020
TypeScript Bot (@typescript-bot) user test this |
Heya Wesley Wigham (@weswigham), I've started to run the parallelized Definitely Typed test suite on this PR at 1ccb136. You can monitor the build here. |
Heya Wesley Wigham (@weswigham), I've started to run the parallelized community code test suite on this PR at 1ccb136. You can monitor the build here. |
Heya Wesley Wigham (@weswigham), I've started to run the extended test suite on this PR at 1ccb136. You can monitor the build here. |
TypeScript Bot (typescript-bot)
commented
Apr 2, 2020
The user suite test run you requested has finished and failed. I've opened a PR with the baseline diff from master. |
Where possible (the conditions being that such an annotation exists, and the type of the annotation produces the same type as what we want to serialize). Fixes#37022.
This causes us to preserve input formatting, whitespace, comments, and aliasing choices.