Skip to content

Commit 7693587

Browse files
clydinmgechev
authored andcommitted
fix(@ngtools/webpack): remove old TypeScript workarounds
These are only needed on older versions of TypeScript which are no longer supported (2.x) and can cause issues with transformers later in the pipeline due to the modification of nodes and potential destructive effects on type analysis.
1 parent 1ca19fb commit 7693587

File tree

1 file changed

+19
-66
lines changed

1 file changed

+19
-66
lines changed

packages/ngtools/webpack/src/transformers/make_transform.ts

+19-66
Original file line numberDiff line numberDiff line change
@@ -16,60 +16,54 @@ import {
1616
TransformOperation,
1717
} from './interfaces';
1818

19-
20-
// Typescript below 2.7.0 needs a workaround.
21-
const tsVersionParts = ts.version.split('.').map(p => Number(p));
22-
const visitEachChild = tsVersionParts[0] <= 2 && tsVersionParts[1] < 7
23-
? visitEachChildWorkaround
24-
: ts.visitEachChild;
25-
2619
export function makeTransform(
2720
standardTransform: StandardTransform,
2821
getTypeChecker?: () => ts.TypeChecker,
2922
): ts.TransformerFactory<ts.SourceFile> {
30-
3123
return (context: ts.TransformationContext): ts.Transformer<ts.SourceFile> => {
3224
const transformer: ts.Transformer<ts.SourceFile> = (sf: ts.SourceFile) => {
3325
const ops: TransformOperation[] = standardTransform(sf);
34-
const removeOps = ops
35-
.filter((op) => op.kind === OPERATION_KIND.Remove) as RemoveNodeOperation[];
36-
const addOps = ops.filter((op) => op.kind === OPERATION_KIND.Add) as AddNodeOperation[];
37-
const replaceOps = ops
38-
.filter((op) => op.kind === OPERATION_KIND.Replace) as ReplaceNodeOperation[];
26+
const removeOps = ops.filter(
27+
op => op.kind === OPERATION_KIND.Remove,
28+
) as RemoveNodeOperation[];
29+
const addOps = ops.filter(op => op.kind === OPERATION_KIND.Add) as AddNodeOperation[];
30+
const replaceOps = ops.filter(
31+
op => op.kind === OPERATION_KIND.Replace,
32+
) as ReplaceNodeOperation[];
3933

4034
// If nodes are removed, elide the imports as well.
4135
// Mainly a workaround for https://github.com/Microsoft/TypeScript/issues/17552.
4236
// WARNING: this assumes that replaceOps DO NOT reuse any of the nodes they are replacing.
4337
// This is currently true for transforms that use replaceOps (replace_bootstrap and
4438
// replace_resources), but may not be true for new transforms.
4539
if (getTypeChecker && removeOps.length + replaceOps.length > 0) {
46-
const removedNodes = removeOps.concat(replaceOps).map((op) => op.target);
40+
const removedNodes = removeOps.concat(replaceOps).map(op => op.target);
4741
removeOps.push(...elideImports(sf, removedNodes, getTypeChecker));
4842
}
4943

50-
const visitor: ts.Visitor = (node) => {
44+
const visitor: ts.Visitor = node => {
5145
let modified = false;
5246
let modifiedNodes = [node];
5347
// Check if node should be dropped.
54-
if (removeOps.find((op) => op.target === node)) {
48+
if (removeOps.find(op => op.target === node)) {
5549
modifiedNodes = [];
5650
modified = true;
5751
}
5852

5953
// Check if node should be replaced (only replaces with first op found).
60-
const replace = replaceOps.find((op) => op.target === node);
54+
const replace = replaceOps.find(op => op.target === node);
6155
if (replace) {
6256
modifiedNodes = [replace.replacement];
6357
modified = true;
6458
}
6559

6660
// Check if node should be added to.
67-
const add = addOps.filter((op) => op.target === node);
61+
const add = addOps.filter(op => op.target === node);
6862
if (add.length > 0) {
6963
modifiedNodes = [
70-
...add.filter((op) => op.before).map(((op) => op.before)),
64+
...add.filter(op => op.before).map(op => op.before),
7165
...modifiedNodes,
72-
...add.filter((op) => op.after).map(((op) => op.after)),
66+
...add.filter(op => op.after).map(op => op.after),
7367
] as ts.Node[];
7468
modified = true;
7569
}
@@ -79,7 +73,7 @@ export function makeTransform(
7973
return modifiedNodes;
8074
} else {
8175
// Otherwise return node as is and visit children.
82-
return visitEachChild(node, visitor, context);
76+
return ts.visitEachChild(node, visitor, context);
8377
}
8478
};
8579

@@ -91,7 +85,7 @@ export function makeTransform(
9185
const result = ts.visitNode(sf, visitor);
9286

9387
// If we removed any decorators, we need to clean up the decorator arrays.
94-
if (removeOps.some((op) => op.target.kind === ts.SyntaxKind.Decorator)) {
88+
if (removeOps.some(op => op.target.kind === ts.SyntaxKind.Decorator)) {
9589
cleanupDecorators(result);
9690
}
9791

@@ -102,52 +96,11 @@ export function makeTransform(
10296
};
10397
}
10498

105-
/**
106-
* This is a version of `ts.visitEachChild` that works that calls our version
107-
* of `updateSourceFileNode`, so that typescript doesn't lose type information
108-
* for property decorators.
109-
* See https://github.com/Microsoft/TypeScript/issues/17384 (fixed by
110-
* https://github.com/Microsoft/TypeScript/pull/20314 and released in TS 2.7.0) and
111-
* https://github.com/Microsoft/TypeScript/issues/17551 (fixed by
112-
* https://github.com/Microsoft/TypeScript/pull/18051 and released on TS 2.5.0).
113-
*/
114-
function visitEachChildWorkaround(
115-
node: ts.Node,
116-
visitor: ts.Visitor,
117-
context: ts.TransformationContext,
118-
) {
119-
120-
if (node.kind === ts.SyntaxKind.SourceFile) {
121-
const sf = node as ts.SourceFile;
122-
const statements = ts.visitLexicalEnvironment(sf.statements, visitor, context);
123-
124-
if (statements === sf.statements) {
125-
return sf;
126-
}
127-
// Note: Need to clone the original file (and not use `ts.updateSourceFileNode`)
128-
// as otherwise TS fails when resolving types for decorators.
129-
const sfClone = ts.getMutableClone(sf);
130-
sfClone.statements = statements;
131-
132-
return sfClone;
133-
}
134-
135-
return ts.visitEachChild(node, visitor, context);
136-
}
137-
138-
139-
// 1) If TS sees an empty decorator array, it will still emit a `__decorate` call.
99+
// If TS sees an empty decorator array, it will still emit a `__decorate` call.
140100
// This seems to be a TS bug.
141-
// 2) Also ensure nodes with modified decorators have parents
142-
// built in TS transformers assume certain nodes have parents (fixed in TS 2.7+)
143101
function cleanupDecorators(node: ts.Node) {
144-
if (node.decorators) {
145-
if (node.decorators.length == 0) {
146-
node.decorators = undefined;
147-
} else if (node.parent == undefined) {
148-
const originalNode = ts.getParseTreeNode(node);
149-
node.parent = originalNode.parent;
150-
}
102+
if (node.decorators && node.decorators.length === 0) {
103+
node.decorators = undefined;
151104
}
152105

153106
ts.forEachChild(node, node => cleanupDecorators(node));

0 commit comments

Comments
 (0)