From a6996327dc18c972a1e11bb0f7a0dc4bf105261c Mon Sep 17 00:00:00 2001 From: Hans Larsen Date: Mon, 19 Jun 2017 12:36:35 -0700 Subject: [PATCH] fix(@ngtools/webpack): support webpack loaders in the webpack plugin This is only useful when using webpack directly. Does not affect the CLI. Performance is going to be impacted, but we do not type check after the loaders. TODO: add typechecking if the source we receive from Webpack is different from the host. --- packages/@ngtools/webpack/src/loader.ts | 28 +++++++++++++++++-- packages/@ngtools/webpack/src/refactor.ts | 15 ++++++---- .../not/so/source/app/app.component.html | 8 ++++++ .../not/so/source/app/app.component.scss | 12 ++++++++ .../not/so/source/app/app.module.ts | 9 ++++++ .../webpack/test-app-weird/package.json | 1 + .../webpack/test-app-weird/webpack.config.js | 15 ++++++++-- .../webpack/test-app-weird/webpack.flags.json | 3 ++ tests/e2e/tests/packages/webpack/weird.ts | 16 +++++++++-- 9 files changed, 95 insertions(+), 12 deletions(-) create mode 100644 tests/e2e/assets/webpack/test-app-weird/webpack.flags.json diff --git a/packages/@ngtools/webpack/src/loader.ts b/packages/@ngtools/webpack/src/loader.ts index 4b220ccd74da..81786238f524 100644 --- a/packages/@ngtools/webpack/src/loader.ts +++ b/packages/@ngtools/webpack/src/loader.ts @@ -422,15 +422,20 @@ function _diagnoseDeps(reasons: ModuleReason[], plugin: AotPlugin, checked: Set< // Super simple TS transpiler loader for testing / isolated usage. does not type check! -export function ngcLoader(this: LoaderContext & { _compilation: any }) { +export function ngcLoader(this: LoaderContext & { _compilation: any }, source: string | null) { const cb = this.async(); const sourceFileName: string = this.resourcePath; const plugin = this._compilation._ngToolsWebpackPluginInstance as AotPlugin; // We must verify that AotPlugin is an instance of the right class. if (plugin && plugin instanceof AotPlugin) { + if (plugin.compilerHost.readFile(sourceFileName) == source) { + // In the case where the source is the same as the one in compilerHost, we don't have + // extra TS loaders and there's no need to do any trickery. + source = null; + } const refactor = new TypeScriptFileRefactor( - sourceFileName, plugin.compilerHost, plugin.program); + sourceFileName, plugin.compilerHost, plugin.program, source); Promise.resolve() .then(() => { @@ -462,6 +467,25 @@ export function ngcLoader(this: LoaderContext & { _compilation: any }) { }); }) .then(() => { + if (source) { + // We need to validate diagnostics. We ignore type checking though, to save time. + const diagnostics = refactor.getDiagnostics(false); + if (diagnostics.length) { + let message = ''; + + diagnostics.forEach(diagnostic => { + const position = diagnostic.file.getLineAndCharacterOfPosition(diagnostic.start); + + const fileName = diagnostic.file.fileName; + const {line, character} = position; + + const messageText = ts.flattenDiagnosticMessageText(diagnostic.messageText, '\n'); + message += `${fileName} (${line + 1},${character + 1}): ${messageText}\n`; + }); + throw new Error(message); + } + } + // Force a few compiler options to make sure we get the result we want. const compilerOptions: ts.CompilerOptions = Object.assign({}, plugin.compilerOptions, { inlineSources: true, diff --git a/packages/@ngtools/webpack/src/refactor.ts b/packages/@ngtools/webpack/src/refactor.ts index 25c85f946415..037a49bf83dc 100644 --- a/packages/@ngtools/webpack/src/refactor.ts +++ b/packages/@ngtools/webpack/src/refactor.ts @@ -38,14 +38,19 @@ export class TypeScriptFileRefactor { constructor(fileName: string, _host: ts.CompilerHost, - private _program?: ts.Program) { + private _program?: ts.Program, + source?: string | null) { fileName = resolve(fileName, _host, _program).replace(/\\/g, '/'); this._fileName = fileName; if (_program) { - this._sourceFile = _program.getSourceFile(fileName); + if (source) { + this._sourceFile = ts.createSourceFile(fileName, source, ts.ScriptTarget.Latest, true); + } else { + this._sourceFile = _program.getSourceFile(fileName); + } } if (!this._sourceFile) { - this._sourceFile = ts.createSourceFile(fileName, _host.readFile(fileName), + this._sourceFile = ts.createSourceFile(fileName, source || _host.readFile(fileName), ts.ScriptTarget.Latest, true); } this._sourceText = this._sourceFile.getFullText(this._sourceFile); @@ -55,7 +60,7 @@ export class TypeScriptFileRefactor { /** * Collates the diagnostic messages for the current source file */ - getDiagnostics(): ts.Diagnostic[] { + getDiagnostics(typeCheck = true): ts.Diagnostic[] { if (!this._program) { return []; } @@ -66,7 +71,7 @@ export class TypeScriptFileRefactor { } diagnostics = diagnostics.concat( this._program.getSyntacticDiagnostics(this._sourceFile), - this._program.getSemanticDiagnostics(this._sourceFile)); + typeCheck ? this._program.getSemanticDiagnostics(this._sourceFile) : []); return diagnostics; } diff --git a/tests/e2e/assets/webpack/test-app-weird/not/so/source/app/app.component.html b/tests/e2e/assets/webpack/test-app-weird/not/so/source/app/app.component.html index 5a532db9308f..3505c196ea74 100644 --- a/tests/e2e/assets/webpack/test-app-weird/not/so/source/app/app.component.html +++ b/tests/e2e/assets/webpack/test-app-weird/not/so/source/app/app.component.html @@ -3,3 +3,11 @@

hello world

lazy + + +

DEBUG_ONLY

+ + + +

PRODUCTION_ONLY

+ diff --git a/tests/e2e/assets/webpack/test-app-weird/not/so/source/app/app.component.scss b/tests/e2e/assets/webpack/test-app-weird/not/so/source/app/app.component.scss index 5cde7b922336..ba143f5d5da1 100644 --- a/tests/e2e/assets/webpack/test-app-weird/not/so/source/app/app.component.scss +++ b/tests/e2e/assets/webpack/test-app-weird/not/so/source/app/app.component.scss @@ -1,3 +1,15 @@ :host { background-color: blue; } + +// @ifdef DEBUG +:host::before { + content: 'DEBUG_ONLY'; +} +// @endif + +// @ifndef DEBUG +:host::before { + content: 'PRODUCTION_ONLY'; +} +// @endif diff --git a/tests/e2e/assets/webpack/test-app-weird/not/so/source/app/app.module.ts b/tests/e2e/assets/webpack/test-app-weird/not/so/source/app/app.module.ts index ded686868a22..2bcd6c4883bd 100644 --- a/tests/e2e/assets/webpack/test-app-weird/not/so/source/app/app.module.ts +++ b/tests/e2e/assets/webpack/test-app-weird/not/so/source/app/app.module.ts @@ -10,6 +10,15 @@ import { AppComponent } from './app.component'; export class HomeView {} +// @ifdef DEBUG +console.log("DEBUG_ONLY"); +// @endif + +// @ifndef DEBUG +console.log("PRODUCTION_ONLY"); +// @endif + + @NgModule({ declarations: [ AppComponent, diff --git a/tests/e2e/assets/webpack/test-app-weird/package.json b/tests/e2e/assets/webpack/test-app-weird/package.json index 40f8f7ba4588..ce162730a1f0 100644 --- a/tests/e2e/assets/webpack/test-app-weird/package.json +++ b/tests/e2e/assets/webpack/test-app-weird/package.json @@ -19,6 +19,7 @@ "devDependencies": { "node-sass": "^3.7.0", "performance-now": "^0.2.0", + "preprocess-loader": "^0.2.2", "raw-loader": "^0.5.1", "sass-loader": "^3.2.0", "typescript": "~2.1.0", diff --git a/tests/e2e/assets/webpack/test-app-weird/webpack.config.js b/tests/e2e/assets/webpack/test-app-weird/webpack.config.js index b1d3de68a4e1..e31c4e7c7c3a 100644 --- a/tests/e2e/assets/webpack/test-app-weird/webpack.config.js +++ b/tests/e2e/assets/webpack/test-app-weird/webpack.config.js @@ -1,5 +1,10 @@ const ngToolsWebpack = require('@ngtools/webpack'); +const flags = require('./webpack.flags.json'); + +const preprocessLoader = 'preprocess-loader' + (flags.DEBUG ? '?+DEBUG' : ''); + + module.exports = { resolve: { extensions: ['.ts', '.js'] @@ -15,10 +20,14 @@ module.exports = { ], module: { loaders: [ - { test: /\.scss$/, loaders: ['raw-loader', 'sass-loader'] }, + { test: /\.scss$/, loaders: ['raw-loader', 'sass-loader', preprocessLoader] }, { test: /\.css$/, loader: 'raw-loader' }, - { test: /\.html$/, loader: 'raw-loader' }, - { test: /\.ts$/, loader: '@ngtools/webpack' } + { test: /\.html$/, loaders: ['raw-loader', preprocessLoader] }, + // Use preprocess to remove DEBUG only code. + { test: /\.ts$/, use: [ + { loader: '@ngtools/webpack' }, + { loader: preprocessLoader } + ] } ] }, devServer: { diff --git a/tests/e2e/assets/webpack/test-app-weird/webpack.flags.json b/tests/e2e/assets/webpack/test-app-weird/webpack.flags.json new file mode 100644 index 000000000000..aac05f738dc8 --- /dev/null +++ b/tests/e2e/assets/webpack/test-app-weird/webpack.flags.json @@ -0,0 +1,3 @@ +{ + "DEBUG": false +} diff --git a/tests/e2e/tests/packages/webpack/weird.ts b/tests/e2e/tests/packages/webpack/weird.ts index 7f5657e1fa7e..353afb7e0a31 100644 --- a/tests/e2e/tests/packages/webpack/weird.ts +++ b/tests/e2e/tests/packages/webpack/weird.ts @@ -1,9 +1,9 @@ import {normalize} from 'path'; import {createProjectFromAsset} from '../../../utils/assets'; -import {exec} from '../../../utils/process'; +import {exec, execWithEnv} from '../../../utils/process'; import {updateJsonFile} from '../../../utils/project'; -import {expectFileSizeToBeUnder, expectFileToExist} from '../../../utils/fs'; +import {expectFileSizeToBeUnder, expectFileToExist, expectFileToMatch} from '../../../utils/fs'; import {expectToFail} from '../../../utils/utils'; @@ -18,10 +18,17 @@ export default function(skipCleaning: () => void) { .then(() => expectFileSizeToBeUnder('dist/app.main.js', 410000)) .then(() => expectFileSizeToBeUnder('dist/0.app.main.js', 40000)) + // Verify that we're using the production environment. + .then(() => expectFileToMatch('dist/app.main.js', /PRODUCTION_ONLY/)) + .then(() => expectToFail(() => expectFileToMatch('dist/app.main.js', /DEBUG_ONLY/))) + // Skip code generation and rebuild. .then(() => updateJsonFile('aotplugin.config.json', json => { json['skipCodeGeneration'] = true; })) + .then(() => updateJsonFile('webpack.flags.json', json => { + json['DEBUG'] = true; + })) .then(() => exec(normalize('node_modules/.bin/webpack'), '-p')) .then(() => expectFileToExist('dist/app.main.js')) .then(() => expectFileToExist('dist/0.app.main.js')) @@ -29,5 +36,10 @@ export default function(skipCleaning: () => void) { .then(() => expectFileToExist('dist/2.app.main.js')) .then(() => expectToFail(() => expectFileSizeToBeUnder('dist/app.main.js', 410000))) .then(() => expectFileSizeToBeUnder('dist/0.app.main.js', 40000)) + + // Verify that we're using the debug environment now. + .then(() => expectFileToMatch('dist/app.main.js', /DEBUG_ONLY/)) + .then(() => expectToFail(() => expectFileToMatch('dist/app.main.js', /PRODUCTION_ONLY/))) + .then(() => skipCleaning()); }