fix(@ngtools/webpack): fix dependency for non-css style factories

filipesilva · hansl · commit 331efeb22cfb · 2017-10-30T10:53:22.000-07:00
diff --git a/packages/@ngtools/webpack/src/angular_compiler_plugin.ts b/packages/@ngtools/webpack/src/angular_compiler_plugin.ts
@@ -828,7 +828,7 @@ export class AngularCompilerPlugin implements Tapable {
         this._emitSkipped = !emitResult || emitResult.emitSkipped;
 
         // Reset changed files on successful compilation.
-        if (this._emitSkipped && this._compilation.errors.length === 0) {
+        if (!this._emitSkipped && this._compilation.errors.length === 0) {
           this._compilerHost.resetChangedFileTracker();
         }
         timeEnd('AngularCompilerPlugin._update');
@@ -872,7 +872,9 @@ export class AngularCompilerPlugin implements Tapable {
         // We also need to all changed files as dependencies of this file, so that all of them
         // will be watched and trigger a rebuild next time.
         outputText = '';
-        errorDependencies = this._getChangedCompilationFiles();
+        errorDependencies = this._getChangedCompilationFiles()
+          // These paths are used by the loader so we must denormalize them.
+          .map((p) => this._compilerHost.denormalizePath(p));
       }
     } else {
       // Check if the TS file exists.
@@ -923,9 +925,14 @@ export class AngularCompilerPlugin implements Tapable {
       .reduce((prev, curr) => prev.concat(curr), [])
       .map((resourcePath) => path.resolve(path.dirname(resolvedFileName), resourcePath))
       .reduce((prev, curr) =>
-        prev.concat(...this._resourceLoader.getResourceDependencies(curr)), []);
+        prev.concat(...this.getResourceDependencies(curr)), []);
 
-    return [...esImports, ...resourceImports];
+    // These paths are meant to be used by the loader so we must denormalize them.
+    return [...esImports, ...resourceImports].map((p) => this._compilerHost.denormalizePath(p));
+  }
+
+  getResourceDependencies(fileName: string): string[] {
+    return this._resourceLoader.getResourceDependencies(fileName);
   }
 
   // This code mostly comes from `performCompilation` in `@angular/compiler-cli`.
diff --git a/packages/@ngtools/webpack/src/compiler_host.ts b/packages/@ngtools/webpack/src/compiler_host.ts
@@ -118,6 +118,10 @@ export class WebpackCompilerHost implements ts.CompilerHost {
     return path.replace(/\\/g, '/');
   }
 
+  denormalizePath(path: string) {
+    return path.replace(/\//g, sep);
+  }
+
   resolve(path: string) {
     path = this._normalizePath(path);
     if (path[0] == '.') {
@@ -161,7 +165,9 @@ export class WebpackCompilerHost implements ts.CompilerHost {
 
   getNgFactoryPaths(): string[] {
     return Object.keys(this._files)
-      .filter(fileName => fileName.endsWith('.ngfactory.js') || fileName.endsWith('.ngstyle.js'));
+      .filter(fileName => fileName.endsWith('.ngfactory.js') || fileName.endsWith('.ngstyle.js'))
+      // These paths are used by the virtual file system decorator so we must denormalize them.
+      .map((path) => this.denormalizePath(path));
   }
 
   invalidate(fileName: string): void {
@@ -291,7 +297,8 @@ export class WebpackCompilerHost implements ts.CompilerHost {
 
   readResource(fileName: string) {
     if (this._resourceLoader) {
-      const denormalizedFileName = fileName.replace(/\//g, sep);
+      // These paths are meant to be used by the loader so we must denormalize them.
+      const denormalizedFileName = this.denormalizePath(fileName);
       const resourceDeps = this._resourceLoader.getResourceDependencies(denormalizedFileName);
 
       if (this._cachedResources[fileName] === undefined
diff --git a/packages/@ngtools/webpack/src/loader.ts b/packages/@ngtools/webpack/src/loader.ts
@@ -581,18 +581,35 @@ export function ngcLoader(this: LoaderContext & { _compilation: any }, source: s
             result.sourceMap = JSON.stringify(sourceMap);
           }
 
-          // Dependencies must use system path separator.
-          // TODO: move the denormalizer into it's own helper.
-          result.errorDependencies.forEach(dep => this.addDependency(dep.replace(/\//g, path.sep)));
-          const dependencies = plugin.getDependencies(sourceFileName);
-          dependencies.forEach(dep => this.addDependency(dep.replace(/\//g, path.sep)));
-
-          // Also add the original file dependencies to virtual files.
-          const virtualFilesRe = /\.(?:ngfactory|css\.shim\.ngstyle)\.js$/;
-          if (virtualFilesRe.test(sourceFileName)) {
-            const originalFile = sourceFileName.replace(virtualFilesRe, '.ts');
+          // Manually add the dependencies for TS files.
+          // Type only imports will be stripped out by compilation so we need to add them as
+          // as dependencies.
+          // Component resources files (html and css templates) also need to be added manually for
+          // AOT, so that this file is reloaded when they change.
+          if (sourceFileName.endsWith('.ts')) {
+            result.errorDependencies.forEach(dep => this.addDependency(dep));
+            const dependencies = plugin.getDependencies(sourceFileName);
+            dependencies.forEach(dep => this.addDependency(dep));
+          }
+
+          // NgFactory files depend on the component template, but we can't know what that file
+          // is (if any). So we add all the dependencies that the original component file has
+          // to the factory as well, which includes html and css templates.
+          const ngFactoryRe = /\.ngfactory.js$/;
+          if (ngFactoryRe.test(sourceFileName)) {
+            const originalFile = sourceFileName.replace(ngFactoryRe, '.ts');
             const origDependencies = plugin.getDependencies(originalFile);
-            origDependencies.forEach(dep => this.addDependency(dep.replace(/\//g, path.sep)));
+            origDependencies.forEach(dep => this.addDependency(dep));
+          }
+
+          // NgStyle files depend on the style file they represent.
+          // E.g. `some-style.less.shim.ngstyle.js` depends on `some-style.less`.
+          // Those files can in turn depend on others, so we have to add them all.
+          const ngStyleRe = /\.shim\.ngstyle\.js$/;
+          if (ngStyleRe.test(sourceFileName)) {
+            const styleFile = sourceFileName.replace(ngStyleRe, '');
+            const styleDependencies = plugin.getResourceDependencies(styleFile);
+            styleDependencies.forEach(dep => this.addDependency(dep));
           }
 
           timeEnd(timeLabel);
diff --git a/packages/@ngtools/webpack/src/virtual_file_system_decorator.ts b/packages/@ngtools/webpack/src/virtual_file_system_decorator.ts
@@ -1,5 +1,4 @@
 import { Stats } from 'fs';
-import { sep } from 'path';
 
 import { InputFileSystem, NodeWatchFileSystemInterface, Callback } from './webpack';
 import { WebpackCompilerHost } from './compiler_host';
@@ -31,8 +30,7 @@ export class VirtualFileSystemDecorator implements InputFileSystem {
   }
 
   getVirtualFilesPaths() {
-    return this._webpackCompilerHost.getNgFactoryPaths()
-      .map((fileName) => fileName.replace(/\//g, sep));
+    return this._webpackCompilerHost.getNgFactoryPaths();
   }
 
   stat(path: string, callback: Callback<any>): void {

Original file line number	Diff line number	Diff line change
`@@ -1,5 +1,4 @@`
`1`	`1`	`import { Stats } from 'fs';`
`2`		`-import { sep } from 'path';`
`3`	`2`
`4`	`3`	`import { InputFileSystem, NodeWatchFileSystemInterface, Callback } from './webpack';`
`5`	`4`	`import { WebpackCompilerHost } from './compiler_host';`
`@@ -31,8 +30,7 @@ export class VirtualFileSystemDecorator implements InputFileSystem {`
`31`	`30`	`}`
`32`	`31`
`33`	`32`	`getVirtualFilesPaths() {`
`34`		`- return this._webpackCompilerHost.getNgFactoryPaths()`
`35`		`- .map((fileName) => fileName.replace(/\//g, sep));`
	`33`	`+ return this._webpackCompilerHost.getNgFactoryPaths();`
`36`	`34`	`}`
`37`	`35`
`38`	`36`	`stat(path: string, callback: Callback<any>): void {`