Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't AMD transform non-module scripts. #146

Merged
merged 1 commit into from
Apr 16, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1,225 changes: 288 additions & 937 deletions packages/analyzer/package-lock.json

Large diffs are not rendered by default.

2 changes: 2 additions & 0 deletions packages/build/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
## Unreleased
* The Babel helpers script now includes all Babel helpers that could be used by the ES5 compilation transform.
* The AMD loader script now includes the Babel helpers needed by the AMD module transform.
* Inline JavaScript will now only be transformed to AMD modules if they have type=module.
* External JavaScript files will now only be transformed to AMD modules if they contain module import/export syntax.
<!-- Add new, unreleased changes here. -->

## [3.0.0-pre.11] - 2018-04-11
Expand Down
8 changes: 8 additions & 0 deletions packages/build/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion packages/build/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
"@babel/preset-es2015": "^7.0.0-beta.42",
"@babel/traverse": "^7.0.0-beta.42",
"@types/babel-types": "^6.25.1",
"@types/babylon": "^6.16.2",
"@types/gulp-if": "0.0.33",
"@types/html-minifier": "^3.5.1",
"@types/is-windows": "^0.2.0",
Expand All @@ -48,6 +49,7 @@
"@types/vinyl": "^2.0.0",
"@types/vinyl-fs": "^2.4.8",
"babel-preset-minify": "=0.4.0-alpha.caaefb4c",
"babylon": "^7.0.0-beta.42",
"css-slam": "^2.1.0",
"dom5": "^3.0.0",
"gulp-if": "^2.0.2",
Expand All @@ -70,7 +72,6 @@
"@types/chai": "^3.4.34",
"@types/fs-extra": "0.0.37",
"@types/mocha": "^2.2.33",
"@types/node": "^6.0.105",
"@types/sinon": "^1.16.33",
"chai": "^4.1.2",
"clang-format": "^1.0.52",
Expand Down
16 changes: 15 additions & 1 deletion packages/build/src/html-splitter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,19 @@ export function scriptWasSplitByHtmlSplitter(script: dom5.Node): boolean {
return dom5.hasAttribute(script, htmlSplitterAttribute);
}

export type HtmlSplitterFile = File&{
fromHtmlSplitter?: true;
moduleScriptIdx?: number
}

/**
* Return whether the given Vinyl file was created by the HtmlSplitter from an
* HTML document script tag.
*/
export function isHtmlSplitterFile(file: File): file is HtmlSplitterFile {
return file.fromHtmlSplitter === true;
}

/**
* Represents a file that is split into multiple files.
*/
Expand Down Expand Up @@ -180,12 +193,13 @@ class HtmlSplitTransform extends AsyncTransformStream<File, File> {
scriptTag.childNodes = [];
dom5.setAttribute(scriptTag, 'src', childFilename);
dom5.setAttribute(scriptTag, htmlSplitterAttribute, '');
const scriptFile = new File({
const scriptFile: HtmlSplitterFile = new File({
cwd: file.cwd,
base: file.base,
path: childPath,
contents: new Buffer(source),
});
scriptFile.fromHtmlSplitter = true;
this._state.addSplitPath(filePath, childPath);
if (isModule) {
scriptFile.moduleScriptIdx = moduleScriptIdx;
Expand Down
106 changes: 72 additions & 34 deletions packages/build/src/js-transform.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
*/

import * as babelCore from '@babel/core';
import * as babylon from 'babylon';
import {relative} from 'path';
import {ModuleResolutionStrategy} from 'polymer-project-config';
import * as uuid from 'uuid/v1';
Expand Down Expand Up @@ -113,11 +114,14 @@ export interface JsTransformOptions {
rootDir?: string;

// Whether to rewrite `import.meta` expressions to objects with inline URLs.
// This transform will always run if the AMD transform runs, regardless of
// this option.
transformImportMeta?: boolean;

// Whether to replace ES modules with AMD modules. Implies
// `transformImportMeta`.
transformModulesToAmd?: boolean;
// Whether to replace ES modules with AMD modules. If `auto`, run the
// transform if the script contains any ES module import/export syntax.
// Implies `transformImportMeta`.
transformModulesToAmd?: boolean|'auto';

// If transformModulesToAmd is true, setting this option will update the
// generated AMD module to be 1) defined with an auto-generated name (instead
Expand All @@ -139,7 +143,7 @@ export function jsTransform(js: string, options: JsTransformOptions): string {
// Even with no transform plugins, parsing and serializing with Babel will
// make some minor formatting changes to the code. Skip Babel altogether
// if we have no meaningful changes to make.
let doBabel = false;
let doBabelTransform = false;

// Note that Babel plugins run in this order:
// 1) plugins, first to last
Expand All @@ -151,12 +155,12 @@ export function jsTransform(js: string, options: JsTransformOptions): string {
plugins.push(babelExternalHelpersPlugin);
}
if (options.minify) {
doBabel = true;
doBabelTransform = true;
// Minify last, so push first.
presets.push(babelPresetMinify);
}
if (options.compileToEs5) {
doBabel = true;
doBabelTransform = true;
presets.push(babelPresetEs2015NoModules);
plugins.push(...babelTransformPlugins);
}
Expand All @@ -165,43 +169,40 @@ export function jsTransform(js: string, options: JsTransformOptions): string {
throw new Error(
'Cannot perform node module resolution without filePath.');
}
doBabel = true;
doBabelTransform = true;
plugins.push(resolveBareSpecifiers(
options.filePath,
!!options.isComponentRequest,
options.packageName,
options.componentDir,
options.rootDir));
}
if (options.transformImportMeta === true ||
(options.transformImportMeta === undefined &&
options.transformModulesToAmd === true)) {
if (!options.filePath) {
throw new Error('Cannot perform importMeta transform without filePath.');
}
if (!options.rootDir) {
throw new Error('Cannot perform importMeta transform without rootDir.');
}
doBabel = true;
let relativeURL = relative(options.rootDir, options.filePath);
if (isWindows()) {
// normalize path separators to URL format
relativeURL = relativeURL.replace(/\\/g, '/');
}
plugins.push(rewriteImportMeta(relativeURL));
}
if (options.transformModulesToAmd) {
if (options.transformImportMeta === false) {
throw new Error(
'Cannot use transformModulesToAmd without transformImportMeta.');
}
doBabel = true;
plugins.push(...babelTransformModulesAmd);

// When the AMD option is "auto", these options will change based on whether
// we have a module or not (unless they are already definitely true).
let transformModulesToAmd = options.transformModulesToAmd;
let transformImportMeta = options.transformImportMeta;
if (transformModulesToAmd === true || transformImportMeta === true) {
doBabelTransform = true;
}

if (doBabel) {
const maybeDoBabelTransform =
doBabelTransform || transformModulesToAmd === 'auto';

if (maybeDoBabelTransform) {
let ast;
try {
js = babelCore.transform(js, {presets, plugins}).code!;
ast = babylon.parse(js, {
// TODO(aomarks) Remove any when typings are updated for babylon 7.
sourceType: transformModulesToAmd === 'auto' ? 'unambiguous' as any :
'module',
plugins: [
'asyncGenerators',
'dynamicImport',
'importMeta' as any,
'objectRestSpread',
],
});
} catch (e) {
if (options.softSyntaxError && e.constructor.name === 'SyntaxError') {
console.error(
Expand All @@ -213,9 +214,46 @@ export function jsTransform(js: string, options: JsTransformOptions): string {
throw e;
}
}

if (transformModulesToAmd === 'auto' &&
ast.program.sourceType === 'module') {
transformModulesToAmd = true;
}

if (transformModulesToAmd) {
doBabelTransform = true;
transformImportMeta = true;
plugins.push(...babelTransformModulesAmd);
}

if (transformImportMeta) {
if (!options.filePath) {
throw new Error(
'Cannot perform importMeta transform without filePath.');
}
if (!options.rootDir) {
throw new Error('Cannot perform importMeta transform without rootDir.');
}
doBabelTransform = true;
let relativeURL = relative(options.rootDir, options.filePath);
if (isWindows()) {
// normalize path separators to URL format
relativeURL = relativeURL.replace(/\\/g, '/');
}
plugins.push(rewriteImportMeta(relativeURL));
}

if (doBabelTransform) {
const result = babelCore.transformFromAst(ast, js, {presets, plugins});
if (result.code === undefined) {
throw new Error(
'Babel transform failed: resulting code was undefined.');
}
js = result.code;
}
}

if (options.transformModulesToAmd && options.moduleScriptIdx !== undefined) {
if (transformModulesToAmd && options.moduleScriptIdx !== undefined) {
const generatedModule = generateModuleName(options.moduleScriptIdx);
const previousGeneratedModule = options.moduleScriptIdx === 0 ?
undefined :
Expand Down
21 changes: 19 additions & 2 deletions packages/build/src/optimize-streams.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import matcher = require('matcher');

import {jsTransform} from './js-transform';
import {htmlTransform} from './html-transform';
import {isHtmlSplitterFile} from './html-splitter';

// TODO(fks) 09-22-2016: Latest npm type declaration resolves to a non-module
// entity. Upgrade to proper JS import once compatible .d.ts file is released,
Expand Down Expand Up @@ -113,16 +114,32 @@ export class JsTransform extends GenericOptimizeTransform {
jsOptions.minify ? notExcluded(jsOptions.minify) : () => false;

const transformer = (content: string, file: File) => {
let transformModulesToAmd: boolean|'auto' = false;
let moduleScriptIdx;

if (jsOptions.transformModulesToAmd) {
if (isHtmlSplitterFile(file)) {
// This is a type=module script in an HTML file. Definitely AMD
// transform.
transformModulesToAmd = file.moduleScriptIdx !== undefined;
moduleScriptIdx = file.moduleScriptIdx;
} else {
// This is an external script file. Only AMD transform it if it looks
// like a module.
transformModulesToAmd = 'auto';
}
}

return jsTransform(content, {
compileToEs5: shouldCompileFile(file),
externalHelpers: true,
minify: shouldMinifyFile(file),
moduleResolution: jsOptions.moduleResolution,
filePath: file.path,
rootDir: options.rootDir,
transformModulesToAmd: jsOptions.transformModulesToAmd,
transformModulesToAmd,
transformImportMeta: jsOptions.transformImportMeta,
moduleScriptIdx: file.moduleScriptIdx,
moduleScriptIdx,
});
};

Expand Down
67 changes: 65 additions & 2 deletions packages/build/src/test/optimize-streams_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -143,20 +143,22 @@ suite('optimize-streams', () => {
});
});

suite('transforms ES modules to AMD with HTML splitter', async () => {
test('inline and external', async () => {
suite('transforms ES modules to AMD', async () => {
test('inline and external script tags', async () => {
const files = [
{
path: 'index.html',
contents: `
<html><head></head><body>
<script>// not a module</script>
<script type="module">import { depA } from './depA.js';</script>
<script type="module" src="./depB.js"></script>
<script type="module">import { depC } from './depC.js';</script>
</body></html>
`,
expected: `
<html><head></head><body>
<script>// not a module</script>
<script>define('polymer-build-generated-module-0', ["./depA.js"], function (_depA) {"use strict";});</script>
<script>define('polymer-build-generated-module-1', ['./depB.js', 'polymer-build-generated-module-0']);</script>
<script>define('polymer-build-generated-module-2', ["./depC.js", 'polymer-build-generated-module-1'], function (_depC) {"use strict";});</script>
Expand Down Expand Up @@ -185,6 +187,67 @@ suite('optimize-streams', () => {
]));
assertMapEqualIgnoringWhitespace(result, expected);
});

test('auto-detects when to transform external js', async () => {
const files = [
{
path: 'has-import-statement.js',
contents: `
import {foo} from './foo.js';
`,
expected: `
define(["./foo.js"], function (_foo) {
"use strict";
});
`,
},

{
path: 'has-export-statement.js',
contents: `
export const foo = 'foo';
`,
expected: `
define(["exports"], function (_exports) {
"use strict";
Object.defineProperty(_exports, "__esModule", {value: true});
_exports.foo = void 0;
const foo = 'foo';
_exports.foo = foo;
});
`,
},

{
path: 'not-a-module.js',
contents: `
const foo = 'import export';
`,
expected: `
const foo = 'import export';
`,
},
];

const opts = {
js: {
transformModulesToAmd: true,
},
rootDir: fixtureRoot,
};

const expected = new Map<string, string>(
files.map((file): [string, string] => [file.path, file.expected]));

const htmlSplitter = new HtmlSplitter();
const result = await getFileMap(pipeStreams([
vfs.src(files),
htmlSplitter.split(),
getOptimizeStreams(opts),
htmlSplitter.rejoin()
]));
assertMapEqualIgnoringWhitespace(result, expected);
});
});

test('minify js', async () => {
Expand Down
2 changes: 2 additions & 0 deletions packages/cli/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
* `build`
* The Babel helpers script now includes all Babel helpers that could be used by the ES5 compilation transform.
* The AMD loader script now includes the Babel helpers needed by the AMD module transform.
* Inline JavaScript will now only be transformed to AMD modules if they have type=module.
* External JavaScript files will now only be transformed to AMD modules if they contain module import/export syntax.
<!-- Add new, unreleased items here. -->

## v1.7.0-pre.10 [04-12-2018]
Expand Down
Loading