Skip to content

Commit

Permalink
fix: Numerous issues with elision due to new TS features in v5+ (fixes
Browse files Browse the repository at this point in the history
  • Loading branch information
nonara committed Feb 19, 2024
1 parent 0bb558e commit 7dc3a4b
Show file tree
Hide file tree
Showing 11 changed files with 3,756 additions and 2,376 deletions.
19 changes: 11 additions & 8 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,11 @@
"release": "standard-version",
"--------------": "",
"format": "prettier --write \"{src,test}/**/{*.js,!(*.d).ts}\"",
"clean": "npx -y rimraf dist **/*.tsbuildinfo ./test/projects/nx/dist",
"clean:all": "yarn run clean && npx -y rimraf node_modules **/node_modules **/yarn.lock yarn.lock",
"clean": "npx -y rimraf -g dist **/*.tsbuildinfo ./test/projects/nx/dist",
"clean:all": "yarn run clean && npx -y rimraf -g node_modules **/node_modules **/yarn.lock yarn.lock",
"reset": "yarn run clean:all && yarn install",
"-------------- ": "",
"prebuild": "npx -y rimraf dist",
"prebuild": "npx -y rimraf -g dist",
"install:tests": "cd test && yarn install",
"prepare": "yarn run install:tests"
},
Expand All @@ -40,9 +40,12 @@
},
"license": "MIT",
"contributors": [
"Daniel Perez Alvarez <danielpza@protonmail.com>",
"Ron S. <ron@nonara.com>"
"Daniel Perez Alvarez <danielpza@protonmail.com>"
],
"author": {
"name": "Ron S.",
"url": "https://twitter.com/Ron"
},
"files": [
"dist",
"types",
Expand All @@ -57,13 +60,13 @@
"@types/node": "^18.11.2",
"jest": "^29.3.1",
"prettier": "^2.7.1",
"rimraf": "^3.0.2",
"rimraf": "^5.0.5",
"standard-version": "^9.5.0",
"@types/ts-expose-internals": "npm:ts-expose-internals@4.9.4",
"ts-jest": "^29.0.3",
"ts-node": "^10.9.1",
"ts-patch": "^2.1.0",
"typescript": "^4.9.4"
"ts-patch": "^3.1.2",
"typescript": "^5.3.3"
},
"peerDependencies": {
"typescript": ">=3.6.5"
Expand Down
152 changes: 127 additions & 25 deletions src/utils/elide-import-export.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,11 @@
/**
* ----------------------------------------------------------------------------
* UPDATE:
*
* TODO - In next major version, we can remove this file entirely due to TS PR 57223
* https://github.com/microsoft/TypeScript/pull/57223
* ----------------------------------------------------------------------------
*
* This file and its contents are due to an issue in TypeScript (affecting *at least* up to 4.1) which causes type
* elision to break during emit for nodes which have been transformed. Specifically, if the 'original' property is set,
* elision functionality no longer works.
Expand All @@ -9,6 +16,7 @@
* the clause with the properly elided information
*
* Issues:
* @see https://github.com/LeDDGroup/typescript-transform-paths/issues/184
* @see https://github.com/microsoft/TypeScript/issues/40603
* @see https://github.com/microsoft/TypeScript/issues/31446
*
Expand All @@ -28,15 +36,21 @@
* import { A, B } from './b'
* export { A } from './b'
*/
import { ImportOrExportClause, ImportOrExportDeclaration, VisitorContext } from "../types";
import { ImportOrExportDeclaration, VisitorContext } from "../types";
import {
ExportDeclaration,
Debug,
EmitResolver,
ExportSpecifier,
ImportClause,
ImportDeclaration,
ImportsNotUsedAsValues,
ImportSpecifier,
isInJSFile,
NamedExportBindings,
NamedExports,
NamedImportBindings,
NamespaceExport,
Node,
StringLiteral,
Visitor,
VisitResult,
} from "typescript";
Expand All @@ -51,19 +65,21 @@ import {
*
* @returns import or export clause or undefined if it entire declaration should be elided
*/
export function elideImportOrExportClause<T extends ImportOrExportDeclaration>(
export function elideImportOrExportDeclaration<T extends ImportOrExportDeclaration>(
context: VisitorContext,
node: T
): (T extends ImportDeclaration ? ImportDeclaration["importClause"] : ExportDeclaration["exportClause"]) | undefined;
node: T,
newModuleSpecifier: StringLiteral,
resolver: EmitResolver
): T | undefined;

export function elideImportOrExportClause(
export function elideImportOrExportDeclaration(
context: VisitorContext,
node: ImportOrExportDeclaration
): ImportOrExportClause | undefined {
const { tsInstance, transformationContext, factory } = context;
const resolver = transformationContext.getEmitResolver();
// Resolver may not be present if run manually (without Program)
if (!resolver) return tsInstance.isImportDeclaration(node) ? node.importClause : node.exportClause;
node: ImportOrExportDeclaration,
newModuleSpecifier: StringLiteral,
resolver: EmitResolver
): ImportOrExportDeclaration | undefined {
const { tsInstance, factory } = context;
const { compilerOptions } = context;

const {
visitNode,
Expand All @@ -72,20 +88,77 @@ export function elideImportOrExportClause(
SyntaxKind,
visitNodes,
isNamedExportBindings,
// 3.8 does not have this, so we have to define it ourselves
// isNamespaceExport,
isIdentifier,
isExportSpecifier,
} = tsInstance;

const isNamespaceExport = tsInstance.isNamespaceExport ?? ((node: Node): node is NamespaceExport => node.kind === SyntaxKind.NamespaceExport);

if (tsInstance.isImportDeclaration(node)) {
if (node.importClause!.isTypeOnly) return undefined;
return visitNode(node.importClause, <Visitor>visitImportClause);
// Do not elide a side-effect only import declaration.
// import "foo";
if (!node.importClause) return node.importClause;

// Always elide type-only imports
if (node.importClause.isTypeOnly) return undefined;

const importClause = visitNode(node.importClause, <Visitor>visitImportClause);

if (
importClause ||
compilerOptions.importsNotUsedAsValues === ImportsNotUsedAsValues.Preserve ||
compilerOptions.importsNotUsedAsValues === ImportsNotUsedAsValues.Error
)
return factory.updateImportDeclaration(
node,
/*modifiers*/ undefined,
importClause,
newModuleSpecifier,
// This will be changed in the next release of TypeScript, but by that point we can drop elision entirely
(node as any).attributes || node.assertClause
);
else return undefined;
} else {
if (node.isTypeOnly) return undefined;
return visitNode(node.exportClause, <Visitor>visitNamedExports, isNamedExportBindings);

if (!node.exportClause || node.exportClause.kind === SyntaxKind.NamespaceExport) {
// never elide `export <whatever> from <whereever>` declarations -
// they should be kept for sideffects/untyped exports, even when the
// type checker doesn't know about any exports
return node;
}

const allowEmpty =
!!compilerOptions.verbatimModuleSyntax ||
(!!node.moduleSpecifier &&
(compilerOptions.importsNotUsedAsValues === ImportsNotUsedAsValues.Preserve ||
compilerOptions.importsNotUsedAsValues === ImportsNotUsedAsValues.Error));

const exportClause = visitNode(
node.exportClause,
<Visitor>((bindings: NamedExportBindings) => visitNamedExportBindings(bindings, allowEmpty)),
isNamedExportBindings
);

return exportClause
? factory.updateExportDeclaration(
node,
/*modifiers*/ undefined,
node.isTypeOnly,
exportClause,
newModuleSpecifier,
// This will be changed in the next release of TypeScript, but by that point we can drop elision entirely
(node as any).attributes || node.assertClause
)
: undefined;
}

/* ********************************************************* *
* Helpers
* ********************************************************* */

// The following visitors are adapted from the TS source-base src/compiler/transformers/ts

/**
Expand All @@ -95,7 +168,7 @@ export function elideImportOrExportClause(
*/
function visitImportClause(node: ImportClause): VisitResult<ImportClause> {
// Elide the import clause if we elide both its name and its named bindings.
const name = resolver.isReferencedAliasDeclaration(node) ? node.name : undefined;
const name = shouldEmitAliasDeclaration(node) ? node.name : undefined;
const namedBindings = visitNode(node.namedBindings, <Visitor>visitNamedImportBindings, isNamedImportBindings);
return name || namedBindings
? factory.updateImportClause(node, /*isTypeOnly*/ false, name, namedBindings)
Expand All @@ -110,11 +183,17 @@ export function elideImportOrExportClause(
function visitNamedImportBindings(node: NamedImportBindings): VisitResult<NamedImportBindings> {
if (node.kind === SyntaxKind.NamespaceImport) {
// Elide a namespace import if it is not referenced.
return resolver.isReferencedAliasDeclaration(node) ? node : undefined;
return shouldEmitAliasDeclaration(node) ? node : undefined;
} else {
// Elide named imports if all of its import specifiers are elided.
const allowEmpty =
compilerOptions.verbatimModuleSyntax ||
(compilerOptions.preserveValueImports &&
(compilerOptions.importsNotUsedAsValues === ImportsNotUsedAsValues.Preserve ||
compilerOptions.importsNotUsedAsValues === ImportsNotUsedAsValues.Error));

const elements = visitNodes(node.elements, <Visitor>visitImportSpecifier, isImportSpecifier);
return tsInstance.some(elements) ? factory.updateNamedImports(node, elements) : undefined;
return allowEmpty || tsInstance.some(elements) ? factory.updateNamedImports(node, elements) : undefined;
}
}

Expand All @@ -125,19 +204,30 @@ export function elideImportOrExportClause(
*/
function visitImportSpecifier(node: ImportSpecifier): VisitResult<ImportSpecifier> {
// Elide an import specifier if it is not referenced.
return resolver.isReferencedAliasDeclaration(node) ? node : undefined;
return !node.isTypeOnly && shouldEmitAliasDeclaration(node) ? node : undefined;
}

/**
* Visits named exports, eliding it if it does not contain an export specifier that
* resolves to a value.
*
* @param node The named exports node.
*/
function visitNamedExports(node: NamedExports): VisitResult<NamedExports> {
function visitNamedExports(node: NamedExports, allowEmpty: boolean): VisitResult<NamedExports> | undefined {
// Elide the named exports if all of its export specifiers were elided.
const elements = visitNodes(node.elements, <Visitor>visitExportSpecifier, isExportSpecifier);
return tsInstance.some(elements) ? factory.updateNamedExports(node, elements) : undefined;
return allowEmpty || tsInstance.some(elements) ? factory.updateNamedExports(node, elements) : undefined;
}

function visitNamedExportBindings(
node: NamedExportBindings,
allowEmpty: boolean
): VisitResult<NamedExportBindings> | undefined {
return isNamespaceExport(node) ? visitNamespaceExports(node) : visitNamedExports(node, allowEmpty);
}

function visitNamespaceExports(node: NamespaceExport): VisitResult<NamespaceExport> {
// Note: This may not work entirely properly, more likely it's just extraneous, but this won't matter soon,
// as we'll be removing elision entirely
return factory.updateNamespaceExport(node, Debug.checkDefined(visitNode(node.name, (n) => n, isIdentifier)));
}

/**
Expand All @@ -147,7 +237,19 @@ export function elideImportOrExportClause(
*/
function visitExportSpecifier(node: ExportSpecifier): VisitResult<ExportSpecifier> {
// Elide an export specifier if it does not reference a value.
return resolver.isValueAliasDeclaration(node) ? node : undefined;
return !node.isTypeOnly && (compilerOptions.verbatimModuleSyntax || resolver.isValueAliasDeclaration(node))
? node
: undefined;
}

function shouldEmitAliasDeclaration(node: Node): boolean {
return (
!!compilerOptions.verbatimModuleSyntax ||
isInJSFile(node) ||
(compilerOptions.preserveValueImports
? resolver.isValueAliasDeclaration(node)
: resolver.isReferencedAliasDeclaration(node))
);
}
}

Expand Down
39 changes: 24 additions & 15 deletions src/visitor.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import ts from "typescript";
import { VisitorContext } from "./types";
import { elideImportOrExportClause, resolvePathAndUpdateNode } from "./utils";
import { elideImportOrExportDeclaration, resolvePathAndUpdateNode } from "./utils";

/* ****************************************************************************************************************** *
* Helpers
Expand Down Expand Up @@ -108,15 +108,16 @@ export function nodeVisitor(this: VisitorContext, node: ts.Node): ts.Node | unde
*/
if (tsInstance.isImportDeclaration(node) && node.moduleSpecifier && tsInstance.isStringLiteral(node.moduleSpecifier))
return resolvePathAndUpdateNode(this, node, node.moduleSpecifier.text, (p) => {
let importClause = node.importClause;

if (!this.isDeclarationFile && importClause?.namedBindings) {
const updatedImportClause = elideImportOrExportClause(this, node);
if (!updatedImportClause) return undefined; // No imports left, elide entire declaration
importClause = updatedImportClause;
// TODO - In next major version, we can remove this entirely due to TS PR 57223
// see: https://github.com/microsoft/TypeScript/pull/57223
// We should at least skip this if doing a minor version update if the ts version is high enough to not need it
if (!this.isDeclarationFile && node.importClause?.namedBindings) {
const resolver = transformationContext.getEmitResolver();
// If run in "manual" mode without a Program, we won't have a resolver, so we can't elide
if (resolver) return elideImportOrExportDeclaration(this, node, p, resolver);
}

return factory.updateImportDeclaration(node, node.modifiers, importClause, p, node.assertClause);
return factory.updateImportDeclaration(node, node.modifiers, node.importClause, p, node.assertClause);
});

/**
Expand All @@ -126,15 +127,23 @@ export function nodeVisitor(this: VisitorContext, node: ts.Node): ts.Node | unde
*/
if (tsInstance.isExportDeclaration(node) && node.moduleSpecifier && tsInstance.isStringLiteral(node.moduleSpecifier))
return resolvePathAndUpdateNode(this, node, node.moduleSpecifier.text, (p) => {
let exportClause = node.exportClause;

if (!this.isDeclarationFile && exportClause && tsInstance.isNamedExports(exportClause)) {
const updatedExportClause = elideImportOrExportClause(this, node);
if (!updatedExportClause) return undefined; // No export left, elide entire declaration
exportClause = updatedExportClause;
// TODO - In next major version, we can remove this entirely due to TS PR 57223
// see: https://github.com/microsoft/TypeScript/pull/57223
// We should at least skip this if doing a minor version update if the ts version is high enough to not need it
if (!this.isDeclarationFile && node.exportClause && tsInstance.isNamedExports(node.exportClause)) {
const resolver = transformationContext.getEmitResolver();
// If run in "manual" mode without a Program, we won't have a resolver, so we can't elide
if (resolver) return elideImportOrExportDeclaration(this, node, p, resolver);
}

return factory.updateExportDeclaration(node, node.modifiers, node.isTypeOnly, exportClause, p, node.assertClause);
return factory.updateExportDeclaration(
node,
node.modifiers,
node.isTypeOnly,
node.exportClause,
p,
node.assertClause
);
});

/**
Expand Down
4 changes: 3 additions & 1 deletion test/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,13 @@
"ts-patch": "link:../node_modules/ts-patch",
"ts-node": "link:../node_modues/ts-node",
"tsp1": "npm:ts-patch@1.*.*",
"tsp2": "npm:ts-patch@2.*.*",
"@nrwl/cli": "^15.0.0",
"@nrwl/js": "^15.0.0",
"@nrwl/node": "^15.0.0",
"@nrwl/workspace": "^15.0.0",
"nx": "^15.0.0"
"nx": "^15.0.0",
"strip-ansi": "^6.0.1"
},
"workspaces": [
"projects/*"
Expand Down
8 changes: 7 additions & 1 deletion test/prepare.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,18 @@ const fs = require("fs");
const path = require("path");
const tsPatch = require("ts-patch");
const tsp1 = require("tsp1");
const tsp2 = require("tsp2");

/* ****************************************************************************************************************** *
* Config
* ****************************************************************************************************************** */

const rootDir = __dirname;
const tsDirs = ["typescript-three", "typescript-four-seven", "typescript"];
const tsDirs = [
"typescript-three",
"typescript-four-seven",
"typescript",
];

/* ****************************************************************************************************************** *
* Patch TS Modules
Expand All @@ -24,4 +29,5 @@ for (const tsDirName of tsDirs) {
// Patch discovered modules
for (const [dirName, dir] of baseDirs)
if (dirName === "typescript-three") tsp1.patch(["tsc.js", "typescript.js"], { basedir: dir });
else if (dirName === "typescript-four-seven") tsp2.patch(["tsc.js", "typescript.js"], { dir });
else tsPatch.patch(["tsc.js", "typescript.js"], { dir });
Loading

0 comments on commit 7dc3a4b

Please sign in to comment.