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
| @@ -3,7 +3,7 @@ class Foo{}; | |||
| >Foo : Foo | |||
|
|
|||
| 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.
| @@ -61,7 +61,7 @@ module m1 { | |||
| } | |||
|
|
|||
| 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.
.../baselines/reference/ClassAndModuleThatMergeWithModuleMemberThatUsesClassTypeParameter.types
Outdated
Show resolved
Hide resolved
|
@typescript-bot user test this |
|
Heya @weswigham, I've started to run the parallelized Definitely Typed test suite on this PR at 1ccb136. You can monitor the build here. |
|
Heya @weswigham, I've started to run the parallelized community code test suite on this PR at 1ccb136. You can monitor the build here. |
|
Heya @weswigham, I've started to run the extended test suite on this PR at 1ccb136. You can monitor the build here. |
|
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.