Skip to content

Commit

Permalink
feat: Use gluing modules for external module support
Browse files Browse the repository at this point in the history
Changed the way we support external modules to what is described in angular/tsickle/issues/1041.

Previously, what we did is described in a previous readme: https://github.com/theseanl/tscc/tree/b0f656e773bc4b43dba6876aa68340f2b5d71dd8#detailed-description-of-external-modules-handling. We replaced names that references the export of an external module. In addition to that, we used some wild hacks that required patching tsickle in order to prevent generation of `goog.requireType()` for external modules.

The way described in the above linked issue requires is simpler and make us free of such hacks. I also vaguely think that this will provide a more correct behavior in case of accessing an external module's global name having side effects.
  • Loading branch information
theseanl committed Sep 18, 2019
2 parents 8b16cc5 + eaa6b1e commit 5591b2a
Show file tree
Hide file tree
Showing 14 changed files with 133 additions and 232 deletions.
4 changes: 2 additions & 2 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
node_modules
/node_modules
/packages/*/node_modules
dist
lerna-debug.log
junit.xml
.tscc_temp
yarn-error.log

9 changes: 7 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,12 @@ Best practice is to provide them as a separate script tag instead of bundling it

1. Users write `import React from 'react'`, so that users' IDE can find necessary typings.
2. TSCC configures tsickle to process type declaration files of module 'react' that Typescript located for us -- usually in node_modules directory.
3. With the help of some Typescript transformers, TSCC removes the above import statements. Doing it requires suppressing `goog.requireType('react')` and substituting `const React = require('react')` to something like `const React_1 = React`.
3. TSCC creates a hypothetical "gluing modules" for each of external modules that looks like
```javascript
goog.module('react')
/** Generated by TSCC */
exports = React
```
4. To inform Closure about such a user-provided global variable name, TSCC generates additional externs for such a global variable, like
```javascript
/**
Expand All @@ -254,7 +259,7 @@ Best practice is to provide them as a separate script tag instead of bundling it
*/
var React = {};
```
tsickle writes module-scoped externs to certain mangled namespace like this, so I am grabbing that namespace to create externs like this. To my understanding, this will provide the required information to Closure, please correct me if I'm wrong.
tsickle writes module-scoped externs to certain mangled namespace like this, so by declaring a global variable to be a type of that namespace, this provides type information of the external module to Closure Compiler.
#### Importing typescript sources in node_modules
Expand Down
48 changes: 0 additions & 48 deletions packages/tscc/src/blacklist_symbol_type.ts

This file was deleted.

46 changes: 46 additions & 0 deletions packages/tscc/src/external_module_support.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
/**
* @fileoverview Transforms `import localName from "external_module"` to
* `const localName = global_name_for_the_external_module`.
* Also transforms `import tslib_any from 'tslib'` to `goog.require("tslib")`.
*/
import ITsccSpecWithTS from './spec/ITsccSpecWithTS';
import {TsickleHost} from 'tsickle';
import {moduleNameAsIdentifier} from 'tsickle/src/annotator_host';

export function getExternsForExternalModules(tsccSpec: ITsccSpecWithTS, tsickleHost: TsickleHost): string {
const moduleNames = tsccSpec.getExternalModuleNames();
const toGlobalName = tsccSpec.getExternalModuleNamesToGlobalsMap();
const header = `\n/** Generated by TSCC */`
let out = '';
for (let moduleName of moduleNames) {
// If a module's type definition is from node_modules, its path is used as a namespace.
// otherwise, it comes from declare module '...' in user-provided files, in which the module name string
// is used as a namespace.
let typeRefFile = tsccSpec.resolveExternalModuleTypeReference(moduleName) || moduleName;
out += `
/**
* @type{${moduleNameAsIdentifier(tsickleHost, typeRefFile)}}
* @const
*/
${tsickleHost.es5Mode ? 'var' : 'const'} ${toGlobalName[moduleName]} = {};\n`;
}
if (out.length) out = header + out;
return out;
}

export function getGluingModules(tsccSpec: ITsccSpecWithTS, tsickleHost: TsickleHost) {
const moduleNames = tsccSpec.getExternalModuleNames();
const toGlobalName = tsccSpec.getExternalModuleNamesToGlobalsMap();
const out: {path: string, content: string}[] = [];
for (let moduleName of moduleNames) {
const content = `goog.module('${moduleName.replace(/([\\'])/g, '\\$1')}')\n` +
`/** Generated by TSCC */\n` +
`exports = ${toGlobalName[moduleName]};`;
// A hypothetical path of this gluing module.
let path = tsccSpec.resolveExternalModuleTypeReference(moduleName) || moduleName;
path = path.replace(/(?:\.d)?\.ts$/, '.js');
out.push({path, content});
}
return out;
}

162 changes: 0 additions & 162 deletions packages/tscc/src/transformer/externalModuleTransformer.ts

This file was deleted.

36 changes: 18 additions & 18 deletions packages/tscc/src/tscc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ import chalk from 'chalk';
import * as StreamArray from 'stream-json/streamers/StreamArray';
import * as tsickle from "tsickle";
import * as ts from "typescript";
import {patchTypeTranslator, registerTypeBlacklistedModuleName} from './blacklist_symbol_type';
import getDefaultLibs from './default_libs';
import {Cache, FSCacheAccessor} from './graph/Cache';
import ClosureDependencyGraph from './graph/ClosureDependencyGraph';
Expand All @@ -18,7 +17,7 @@ import spawnCompiler from './spawn_compiler';
import ITsccSpecWithTS from "./spec/ITsccSpecWithTS";
import TsccSpecWithTS, {TsError} from "./spec/TsccSpecWithTS";
import decoratorPropertyTransformer from './transformer/decoratorPropertyTransformer';
import externalModuleTransformer, {getExternsForExternalModules} from './transformer/externalModuleTransformer';
import {getExternsForExternalModules, getGluingModules} from './external_module_support';
import fs = require('fs');
import path = require('path');
import stream = require('stream');
Expand All @@ -42,19 +41,19 @@ export default async function tscc(
tsccSpecJSONOrItsPath: string | IInputTsccSpecJSON,
tsConfigPathOrTsArgs?: string,
compilerOptionsOverride?: object
):Promise<void>
): Promise<void>
/** @internal */
export default async function tscc(
tsccSpecJSONOrItsPath: string | IInputTsccSpecJSON,
tsConfigPathOrTsArgs: string[],
compilerOptionsOverride?: object
):Promise<void>
): Promise<void>
/** @internal */
export default async function tscc(
tsccSpecJSONOrItsPath: string | IInputTsccSpecJSON,
tsConfigPathOrTsArgs?: string | string[],
compilerOptionsOverride?: object
):Promise<void> {
): Promise<void> {
const tsccLogger = new Logger(chalk.green("TSCC: "), process.stderr);
const tsLogger = new Logger(chalk.blue("TS: "), process.stderr);

Expand Down Expand Up @@ -115,11 +114,16 @@ export default async function tscc(
writeFile(path, fs.readFileSync(path, 'utf8'))
})

// Manually push gluing modules
getGluingModules(tsccSpec, transformerHost).forEach(({path, content}) => {
writeFile(path, content)
})

patchTsickleResolveModule(); // check comments in its source - required to generate proper externs
const result = tsickle.emit(program, transformerHost, writeFile, undefined, undefined, false, {
afterTs: [
decoratorPropertyTransformer(transformerHost),
externalModuleTransformer(tsccSpec, transformerHost, program.getTypeChecker())
//externalModuleTransformer(tsccSpec, transformerHost, program.getTypeChecker())
]
});
// If tsickle errors, print diagnostics and exit.
Expand Down Expand Up @@ -303,26 +307,17 @@ function getTsickleHost(tsccSpec: ITsccSpecWithTS, logger: Logger): tsickle.Tsic
const externalModuleNames = tsccSpec.getExternalModuleNames();
const resolvedExternalModuleTypeRefs: string[] = [];

let hasTypeBlacklistedSymbols = false;
for (let i = 0, l = externalModuleNames.length; i < l; i++) {
let name = externalModuleNames[i];
for (let name of externalModuleNames) {
let typeRefFileName = tsccSpec.resolveExternalModuleTypeReference(name);
if (typeRefFileName) {
resolvedExternalModuleTypeRefs.push(typeRefFileName);
} else {
// Can't achieve blacklisting via TsickleHost.typeBlacklistPaths API
hasTypeBlacklistedSymbols = true;
registerTypeBlacklistedModuleName(name);
}
}
if (hasTypeBlacklistedSymbols) {
patchTypeTranslator();
}

const externalModuleRoots = resolvedExternalModuleTypeRefs
.map(getPackageBoundary);

const ignoreWarningsPath = tsccSpec.debug().ignoreWarningsPath || ["/node_modules"];
const ignoreWarningsPath = tsccSpec.debug().ignoreWarningsPath || ["/node_modules/"];

const transformerHost: tsickle.TsickleHost = {
shouldSkipTsickleProcessing(fileName: string) {
Expand All @@ -344,7 +339,7 @@ function getTsickleHost(tsccSpec: ITsccSpecWithTS, logger: Logger): tsickle.Tsic
googmodule: true,
transformDecorators: true,
transformTypesToClosure: true,
typeBlackListPaths: new Set(resolvedExternalModuleTypeRefs),
typeBlackListPaths: new Set(),
untyped: false,
logWarning(warning) {
if (warning.file) {
Expand All @@ -369,6 +364,11 @@ function getTsickleHost(tsccSpec: ITsccSpecWithTS, logger: Logger): tsickle.Tsic
* deterministic output based on a single file.
*/
pathToModuleName: (context: string, fileName: string) => {
// Module names specified as external are not resolved, which in effect cause
// googmodule transformer to emit module names verbatim in `goog.require()`.
if (externalModuleNames.includes(fileName)) return fileName;
// 'tslib' is always considered as an external module.
if (fileName === 'tslib') return fileName;
// Resolve module via ts API
const resolved = ts.resolveModuleName(fileName, context, options, compilerHost);
if (resolved && resolved.resolvedModule) {
Expand Down
Loading

0 comments on commit 5591b2a

Please sign in to comment.