Skip to content

Commit

Permalink
Changed the way we support external modules to what is described in a…
Browse files Browse the repository at this point in the history
…ngular/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
1 parent 8b16cc5 commit c54c37f
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 c54c37f

Please sign in to comment.