Unable to publish due to baseline difference in .d.ts emit

[DanielRosenwasser]

We seem to be hitting some sort of issue with parenthesization on .d.ts files. This has been blocking nightly publishes, and will block any sort of beta publish next week.

https://typescript.visualstudio.com/TypeScript/_build/results?buildId=76918&view=logs&j=fd490c07-0b22-5182-fac9-6d67fe1e939b&t=00933dce-c782-5c03-4a85-76379ccfa50a&l=139

[weswigham]

@rbuckton is this an intended outcome of the node factory work? Should we LKG and accept the difference?

[rbuckton]

It could be. Odd though that this didn't show up in the API tests before now.

[weswigham]

Iirc, the API tests are consistently captured with the lib compiler, so it doesn't appear until you do an LKG (as is done for nightlies and publishes).

[rbuckton]

I'm not entirely certain why this changed. factory.createUnionTypeNode calls parenthesizer.parenthesizeConstituentTypesOfUnionOrIntersectionType(types) which should do the same thing it did prior to moving the API: when unioning A & B with C it should be parenthesizing it as (A & B) | C since our parenthesizing logic for types is fairly aggressive and doesn't take precedence into account.

[weswigham]

It's very likely the parenthesizer was never being invoked before, as those return type nodes are normally just copied from the source (since it's an interface)

[sheetalkamat]

@weswigham if it was copied from source then it should be copied from source now as well?

[weswigham]

It should be! The parenthesizer in the factory may have had other ideas when the updateUnionType method was fed back in the input nodes, though!

[rbuckton]

Ah, I discovered the difference, and accepting the change should be fine. In 3.9 we called parenthesizeTypeParameters in declarations.ts when emitting a type reference:

TypeScript/src/compiler/transformers/declarations.ts

Lines 864 to 868 in 986e9dd

	case SyntaxKind.TypeReference: {
	checkEntityNameVisibility(input.typeName, enclosingDeclaration);
	const node = visitEachChild(input, visitDeclarationSubtree, context);
	return cleanup(updateTypeReferenceNode(node, node.typeName, parenthesizeTypeParameters(node.typeArguments)));
	}

And parenthesizeTypeParameters would unconditionally create a new NodeArray even if there were no changes:

TypeScript/src/compiler/factory.ts

Lines 1274 to 1286 in 986e9dd

	export function parenthesizeTypeParameters(typeParameters: readonly TypeNode[] | undefined) {
	if (some(typeParameters)) {
	const params: TypeNode[] = [];
	for (let i = 0; i < typeParameters.length; ++i) {
	const entry = typeParameters[i];
	params.push(i === 0 && isFunctionOrConstructorTypeNode(entry) && entry.typeParameters ?
	createParenthesizedType(entry) :
	entry);
	}
	return createNodeArray(params);
	}
	}

NOTE: It's not the call to createNodeArray but rather the initialization of params to a new array here:

TypeScript/src/compiler/factory.ts

Line 1276 in 986e9dd

const params: TypeNode[] = [];

By returning a new NodeArray here, we end up creating new nodes starting at NodeArray<TypeNode> in the union SignatureDeclaration & {typeArguments?: NodeArray<any>} | undefined, which triggers us to hit our over-aggressive parenthesization logic.

One of the node factory changes was to move the parenthesizeTypeParameters logic into the factory, as we do with all other parenthesis rules:

TypeScript/src/compiler/factory/nodeFactory.ts

Lines 1626 to 1632 in fe33e61

	function createTypeReferenceNode(typeName: string | EntityName, typeArguments: readonly TypeNode[] | undefined) {
	const node = createBaseNode<TypeReferenceNode>(SyntaxKind.TypeReference);
	node.typeName = asName(typeName);
	node.typeArguments = typeArguments && parenthesizerRules().parenthesizeTypeArguments(createNodeArray(typeArguments));
	node.transformFlags = TransformFlags.ContainsTypeScript;
	return node;
	}

In the updated parenthesizer rules, we use sameMap instead of allocating a new array:

TypeScript/src/compiler/factory/parenthesizerRules.ts

Lines 398 to 402 in fe33e61

	function parenthesizeTypeArguments(typeArguments: NodeArray<TypeNode> | undefined): NodeArray<TypeNode> | undefined {
	if (some(typeArguments)) {
	return factory.createNodeArray(sameMap(typeArguments, parenthesizeOrdinalTypeArgument));
	}
	}

This results in us just reusing the original node since there are no changes in the type reference.

[rbuckton]

I just pushed up #39173 which runs LKG and accepts the new API baselines.

[rbuckton]

Publish is working again: