Skip to content

Commit

Permalink
fix(@angular-devkit/build-angular): suppress warning for CommonJS tem…
Browse files Browse the repository at this point in the history
…plateUrl and styleUrl

Currently, when users use either absolute paths, or path mappings in JIT mode, we issue a warning for templateUrl and styleUrl. With this change we suppress those warning.

Closes: #18057
(cherry picked from commit 2aedad9)
  • Loading branch information
alan-agius4 authored and clydin committed Jul 15, 2020
1 parent ea907d7 commit fcfcd68
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ import { Compiler, compilation } from 'webpack';
const CommonJsRequireDependency = require('webpack/lib/dependencies/CommonJsRequireDependency');
const AMDDefineDependency = require('webpack/lib/dependencies/AMDDefineDependency');

const STYLES_TEMPLATE_URL_REGEXP = /\.(html|svg|css|sass|less|styl|scss)$/;

// The below is extended because there are not in the typings
interface WebpackModule extends compilation.Module {
name?: string;
Expand All @@ -23,6 +25,10 @@ interface WebpackModule extends compilation.Module {
userRequest?: string;
}

interface CommonJsRequireDependencyType {
request: string;
}

export interface CommonJsUsageWarnPluginOptions {
/** A list of CommonJS packages that are allowed to be used without a warning. */
allowedDepedencies?: string[];
Expand Down Expand Up @@ -61,7 +67,7 @@ export class CommonJsUsageWarnPlugin {
continue;
}

if (this.hasCommonJsDependencies(dependencies)) {
if (this.hasCommonJsDependencies(dependencies, true)) {
// Dependency is CommonsJS or AMD.

// Check if it's parent issuer is also a CommonJS dependency.
Expand Down Expand Up @@ -97,8 +103,23 @@ export class CommonJsUsageWarnPlugin {
});
}

private hasCommonJsDependencies(dependencies: unknown[]): boolean {
return dependencies.some(d => d instanceof CommonJsRequireDependency || d instanceof AMDDefineDependency);
private hasCommonJsDependencies(dependencies: unknown[], checkForStylesAndTemplatesCJS = false): boolean {
for (const dep of dependencies) {
if (dep instanceof CommonJsRequireDependency) {
if (checkForStylesAndTemplatesCJS && STYLES_TEMPLATE_URL_REGEXP.test((dep as CommonJsRequireDependencyType).request)) {
// Skip in case it's a template or stylesheet
continue;
}

return true;
}

if (dep instanceof AMDDefineDependency) {
return true;
}
}

return false;
}

private rawRequestToPackageName(rawRequest: string): string {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,4 +77,24 @@ describe('Browser Builder commonjs warning', () => {
expect(logs.join()).not.toContain('WARNING');
await run.stop();
});

it('should not show warning in JIT for templateUrl and styleUrl when using paths', async () => {
host.replaceInFile('tsconfig.base.json', /"baseUrl": ".\/",/, `
"baseUrl": "./",
"paths": {
"@app/*": [
"src/app/*"
]
},
`);

host.replaceInFile('src/app/app.module.ts', './app.component', '@app/app.component');

const run = await architect.scheduleTarget(targetSpec, { aot: false }, { logger });
const output = await run.result;
expect(output.success).toBe(true);

expect(logs.join()).not.toContain('WARNING');
await run.stop();
});
});

0 comments on commit fcfcd68

Please sign in to comment.