Skip to content

Commit

Permalink
fix(@ngtools/webpack): support webpack loaders in the webpack plugin
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
hansl authored and Brocco committed Jun 21, 2017
1 parent 02029cf commit a699632
Show file tree
Hide file tree
Showing 9 changed files with 95 additions and 12 deletions.
28 changes: 26 additions & 2 deletions packages/@ngtools/webpack/src/loader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(() => {
Expand Down Expand Up @@ -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,
Expand Down
15 changes: 10 additions & 5 deletions packages/@ngtools/webpack/src/refactor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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 [];
}
Expand All @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,11 @@ <h1>hello world</h1>
<a [routerLink]="['lazy']">lazy</a>
<router-outlet></router-outlet>
</div>

<!-- @ifdef DEBUG -->
<p>DEBUG_ONLY</p>
<!-- @endif -->

<!-- @ifndef DEBUG -->
<p>PRODUCTION_ONLY</p>
<!-- @endif -->
Original file line number Diff line number Diff line change
@@ -1,3 +1,15 @@
:host {
background-color: blue;
}

// @ifdef DEBUG
:host::before {
content: 'DEBUG_ONLY';
}
// @endif

// @ifndef DEBUG
:host::before {
content: 'PRODUCTION_ONLY';
}
// @endif
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
1 change: 1 addition & 0 deletions tests/e2e/assets/webpack/test-app-weird/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
15 changes: 12 additions & 3 deletions tests/e2e/assets/webpack/test-app-weird/webpack.config.js
Original file line number Diff line number Diff line change
@@ -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']
Expand All @@ -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: {
Expand Down
3 changes: 3 additions & 0 deletions tests/e2e/assets/webpack/test-app-weird/webpack.flags.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"DEBUG": false
}
16 changes: 14 additions & 2 deletions tests/e2e/tests/packages/webpack/weird.ts
Original file line number Diff line number Diff line change
@@ -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';


Expand All @@ -18,16 +18,28 @@ 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'))
.then(() => expectFileToExist('dist/1.app.main.js'))
.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());
}

0 comments on commit a699632

Please sign in to comment.