From 1ccce7dcf70f28153b81f71b37d43423737d83b1 Mon Sep 17 00:00:00 2001 From: Alan Agius Date: Tue, 30 Nov 2021 11:06:35 +0100 Subject: [PATCH 1/2] refactor(@ngtools/webpack): remove direct angular resource loader We remove the custom direct resource loader which is used for JIT when `directTemplateLoading` is enabled. Instead, use Webpack's [asset modules](https://webpack.js.org/guides/asset-modules/) which were introduced in version 5. To the resource URL, we also add a query parameter, `ngResource`. This is used to be filter request based on a query. See https://webpack.js.org/guides/asset-modules/#replacing-inline-loader-syntax for more information. --- .../src/webpack/configs/common.ts | 6 ++ .../ngtools/webpack/src/ivy/transformation.ts | 8 +- .../webpack/src/loaders/direct-resource.ts | 19 ---- .../ngtools/webpack/src/resource_loader.ts | 28 ++++-- .../src/transformers/replace_resources.ts | 27 +++--- .../transformers/replace_resources_spec.ts | 91 +++++-------------- 6 files changed, 58 insertions(+), 121 deletions(-) delete mode 100644 packages/ngtools/webpack/src/loaders/direct-resource.ts diff --git a/packages/angular_devkit/build_angular/src/webpack/configs/common.ts b/packages/angular_devkit/build_angular/src/webpack/configs/common.ts index a72944f69053..f6b098f965b3 100644 --- a/packages/angular_devkit/build_angular/src/webpack/configs/common.ts +++ b/packages/angular_devkit/build_angular/src/webpack/configs/common.ts @@ -375,6 +375,12 @@ export async function getCommonConfig(wco: WebpackConfigOptions): Promise true, - getTypeChecker, - options.directTemplateLoading, - options.inlineStyleFileExtension, - ), + replaceResources(() => true, getTypeChecker, options.inlineStyleFileExtension), compilerCli.constructorParametersDownlevelTransform(builder.getProgram()), ], }; diff --git a/packages/ngtools/webpack/src/loaders/direct-resource.ts b/packages/ngtools/webpack/src/loaders/direct-resource.ts deleted file mode 100644 index 5c5efda6c7c9..000000000000 --- a/packages/ngtools/webpack/src/loaders/direct-resource.ts +++ /dev/null @@ -1,19 +0,0 @@ -/** - * @license - * Copyright Google LLC All Rights Reserved. - * - * Use of this source code is governed by an MIT-style license that can be - * found in the LICENSE file at https://angular.io/license - */ - -import { LoaderContext } from 'webpack'; - -export const DirectAngularResourceLoaderPath = __filename; - -export default function (this: LoaderContext<{ esModule?: 'true' | 'false' }>, content: string) { - const { esModule } = this.getOptions(); - - return `${esModule === 'false' ? 'module.exports =' : 'export default'} ${JSON.stringify( - content, - )};`; -} diff --git a/packages/ngtools/webpack/src/resource_loader.ts b/packages/ngtools/webpack/src/resource_loader.ts index 9be817676e8a..fe79c74bcf11 100644 --- a/packages/ngtools/webpack/src/resource_loader.ts +++ b/packages/ngtools/webpack/src/resource_loader.ts @@ -16,6 +16,7 @@ import { InlineAngularResourceLoaderPath, InlineAngularResourceSymbol, } from './loaders/inline-resource'; +import { NG_COMPONENT_RESOURCE_QUERY } from './transformers/replace_resources'; interface CompilationOutput { content: string; @@ -110,17 +111,24 @@ export class WebpackResourceLoader { throw new Error('WebpackResourceLoader cannot be used without parentCompilation'); } - // Create a special URL for reading the resource from memory - const entry = - filePath || - (resourceType - ? `${containingFile}-${this.outputPathCounter}.${fileExtension}!=!${this.inlineDataLoaderPath}!${containingFile}` - : // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - `angular-resource:${resourceType},${createHash('md5').update(data!).digest('hex')}`); + const getEntry = (): string => { + if (filePath) { + return `${filePath}?${NG_COMPONENT_RESOURCE_QUERY}`; + } else if (resourceType) { + return ( + // app.component.ts-2.css?ngResource!=!@ngtools/webpack/src/loaders/inline-resource.js!app.component.ts + `${containingFile}-${this.outputPathCounter}.${fileExtension}` + + `?${NG_COMPONENT_RESOURCE_QUERY}!=!${this.inlineDataLoaderPath}!${containingFile}` + ); + } else if (data) { + // Create a special URL for reading the resource from memory + return `angular-resource:${resourceType},${createHash('md5').update(data).digest('hex')}`; + } - if (!entry) { - throw new Error(`"filePath" or "data" must be specified.`); - } + throw new Error(`"filePath", "resourceType" or "data" must be specified.`); + }; + + const entry = getEntry(); // Simple sanity check. if (filePath?.match(/\.[jt]s$/)) { diff --git a/packages/ngtools/webpack/src/transformers/replace_resources.ts b/packages/ngtools/webpack/src/transformers/replace_resources.ts index a4f47c78c805..36ddef728a43 100644 --- a/packages/ngtools/webpack/src/transformers/replace_resources.ts +++ b/packages/ngtools/webpack/src/transformers/replace_resources.ts @@ -7,13 +7,13 @@ */ import * as ts from 'typescript'; -import { DirectAngularResourceLoaderPath } from '../loaders/direct-resource'; import { InlineAngularResourceLoaderPath } from '../loaders/inline-resource'; +export const NG_COMPONENT_RESOURCE_QUERY = 'ngResource'; + export function replaceResources( shouldTransform: (fileName: string) => boolean, getTypeChecker: () => ts.TypeChecker, - directTemplateLoading = false, inlineStyleFileExtension?: string, ): ts.TransformerFactory { return (context: ts.TransformationContext) => { @@ -30,7 +30,6 @@ export function replaceResources( nodeFactory, node, typeChecker, - directTemplateLoading, resourceImportDeclarations, moduleKind, inlineStyleFileExtension, @@ -81,7 +80,6 @@ function visitDecorator( nodeFactory: ts.NodeFactory, node: ts.Decorator, typeChecker: ts.TypeChecker, - directTemplateLoading: boolean, resourceImportDeclarations: ts.ImportDeclaration[], moduleKind?: ts.ModuleKind, inlineStyleFileExtension?: string, @@ -111,7 +109,6 @@ function visitDecorator( nodeFactory, node, styleReplacements, - directTemplateLoading, resourceImportDeclarations, moduleKind, inlineStyleFileExtension, @@ -144,7 +141,6 @@ function visitComponentMetadata( nodeFactory: ts.NodeFactory, node: ts.ObjectLiteralElementLike, styleReplacements: ts.Expression[], - directTemplateLoading: boolean, resourceImportDeclarations: ts.ImportDeclaration[], moduleKind: ts.ModuleKind = ts.ModuleKind.ES2015, inlineStyleFileExtension?: string, @@ -159,11 +155,7 @@ function visitComponentMetadata( return undefined; case 'templateUrl': - const loaderOptions = moduleKind < ts.ModuleKind.ES2015 ? '?esModule=false' : ''; - const url = getResourceUrl( - node.initializer, - directTemplateLoading ? `!${DirectAngularResourceLoaderPath}${loaderOptions}!` : '', - ); + const url = getResourceUrl(node.initializer); if (!url) { return node; } @@ -200,9 +192,12 @@ function visitComponentMetadata( if (inlineStyleFileExtension) { const data = Buffer.from(node.text).toString('base64'); const containingFile = node.getSourceFile().fileName; - url = `${containingFile}.${inlineStyleFileExtension}!=!${InlineAngularResourceLoaderPath}?data=${encodeURIComponent( - data, - )}!${containingFile}`; + // app.component.ts.css?ngResource!=!@ngtools/webpack/src/loaders/inline-resource.js?data=...!app.component.ts + url = + `${containingFile}.${inlineStyleFileExtension}?${NG_COMPONENT_RESOURCE_QUERY}` + + `!=!${InlineAngularResourceLoaderPath}?data=${encodeURIComponent( + data, + )}!${containingFile}`; } else { return nodeFactory.createStringLiteral(node.text); } @@ -230,13 +225,13 @@ function visitComponentMetadata( } } -export function getResourceUrl(node: ts.Node, loader = ''): string | null { +export function getResourceUrl(node: ts.Node): string | null { // only analyze strings if (!ts.isStringLiteral(node) && !ts.isNoSubstitutionTemplateLiteral(node)) { return null; } - return `${loader}${/^\.?\.\//.test(node.text) ? '' : './'}${node.text}`; + return `${/^\.?\.\//.test(node.text) ? '' : './'}${node.text}?${NG_COMPONENT_RESOURCE_QUERY}`; } function isComponentDecorator(node: ts.Node, typeChecker: ts.TypeChecker): node is ts.Decorator { diff --git a/packages/ngtools/webpack/src/transformers/replace_resources_spec.ts b/packages/ngtools/webpack/src/transformers/replace_resources_spec.ts index 0f09e6f7283a..8647f60957a9 100644 --- a/packages/ngtools/webpack/src/transformers/replace_resources_spec.ts +++ b/packages/ngtools/webpack/src/transformers/replace_resources_spec.ts @@ -8,14 +8,12 @@ import { tags } from '@angular-devkit/core'; import * as ts from 'typescript'; -import { DirectAngularResourceLoaderPath } from '../loaders/direct-resource'; import { replaceResources } from './replace_resources'; import { createTypescriptContext, transformTypescript } from './spec_helpers'; function transform( input: string, shouldTransform = true, - directTemplateLoading = true, importHelpers = true, module: ts.ModuleKind = ts.ModuleKind.ES2020, ) { @@ -24,11 +22,7 @@ function transform( module, }); const getTypeChecker = () => program.getTypeChecker(); - const transformer = replaceResources( - () => shouldTransform, - getTypeChecker, - directTemplateLoading, - ); + const transformer = replaceResources(() => shouldTransform, getTypeChecker); return transformTypescript(input, [transformer], program, compilerHost); } @@ -51,9 +45,9 @@ describe('@ngtools/webpack transformers', () => { `; const output = tags.stripIndent` import { __decorate } from "tslib"; - import __NG_CLI_RESOURCE__0 from "!${DirectAngularResourceLoaderPath}!./app.component.html"; - import __NG_CLI_RESOURCE__1 from "./app.component.css"; - import __NG_CLI_RESOURCE__2 from "./app.component.2.css"; + import __NG_CLI_RESOURCE__0 from "./app.component.html?ngResource"; + import __NG_CLI_RESOURCE__1 from "./app.component.css?ngResource"; + import __NG_CLI_RESOURCE__2 from "./app.component.2.css?ngResource"; import { Component } from '@angular/core'; let AppComponent = class AppComponent { @@ -102,53 +96,12 @@ describe('@ngtools/webpack transformers', () => { AppComponent = (0, tslib_1.__decorate)([ (0, core_1.Component)({ selector: 'app-root', - template: require("!${DirectAngularResourceLoaderPath}?esModule=false!./app.component.html"), - styles: [require("./app.component.css"), require("./app.component.2.css")] }) ], AppComponent); + template: require("./app.component.html?ngResource"), + styles: [require("./app.component.css?ngResource"), require("./app.component.2.css?ngResource")] }) ], AppComponent); exports.AppComponent = AppComponent; `; - const result = transform(input, true, true, true, ts.ModuleKind.CommonJS); - expect(tags.oneLine`${result}`).toEqual(tags.oneLine`${output}`); - }); - - it('should not replace resources when directTemplateLoading is false', () => { - const input = tags.stripIndent` - import { Component } from '@angular/core'; - - @Component({ - selector: 'app-root', - templateUrl: './app.component.html', - styleUrls: [ - './app.component.css', - './app.component.2.css' - ] - }) - export class AppComponent { - title = 'app'; - } - `; - const output = tags.stripIndent` - import { __decorate } from "tslib"; - import __NG_CLI_RESOURCE__0 from "./app.component.html"; - import __NG_CLI_RESOURCE__1 from "./app.component.css"; - import __NG_CLI_RESOURCE__2 from "./app.component.2.css"; - import { Component } from '@angular/core'; - let AppComponent = class AppComponent { - constructor() { - this.title = 'app'; - } - }; - AppComponent = __decorate([ - Component({ - selector: 'app-root', - template: __NG_CLI_RESOURCE__0, - styles: [__NG_CLI_RESOURCE__1, __NG_CLI_RESOURCE__2] - }) - ], AppComponent); - export { AppComponent }; - `; - - const result = transform(input, true, false); + const result = transform(input, true, true, ts.ModuleKind.CommonJS); expect(tags.oneLine`${result}`).toEqual(tags.oneLine`${output}`); }); @@ -166,7 +119,7 @@ describe('@ngtools/webpack transformers', () => { `; const output = tags.stripIndent` import { __decorate } from "tslib"; - import __NG_CLI_RESOURCE__0 from "!${DirectAngularResourceLoaderPath}!./app.component.svg"; + import __NG_CLI_RESOURCE__0 from "./app.component.svg?ngResource"; import { Component } from '@angular/core'; let AppComponent = class AppComponent { constructor() { @@ -202,8 +155,8 @@ describe('@ngtools/webpack transformers', () => { `; const output = tags.stripIndent` import { __decorate } from "tslib"; - import __NG_CLI_RESOURCE__0 from "!${DirectAngularResourceLoaderPath}!./app.component.html"; - import __NG_CLI_RESOURCE__1 from "./app.component.css"; + import __NG_CLI_RESOURCE__0 from "./app.component.html?ngResource"; + import __NG_CLI_RESOURCE__1 from "./app.component.css?ngResource"; import { Component } from '@angular/core'; let AppComponent = class AppComponent { @@ -240,9 +193,9 @@ describe('@ngtools/webpack transformers', () => { `; const output = ` import { __decorate } from "tslib"; - import __NG_CLI_RESOURCE__0 from "!${DirectAngularResourceLoaderPath}!./app.component.html"; - import __NG_CLI_RESOURCE__1 from "./app.component.css"; - import __NG_CLI_RESOURCE__2 from "./app.component.2.css"; + import __NG_CLI_RESOURCE__0 from "./app.component.html?ngResource"; + import __NG_CLI_RESOURCE__1 from "./app.component.css?ngResource"; + import __NG_CLI_RESOURCE__2 from "./app.component.2.css?ngResource"; import { Component } from '@angular/core'; let AppComponent = class AppComponent { @@ -279,9 +232,9 @@ describe('@ngtools/webpack transformers', () => { `; const output = tags.stripIndent` import { __decorate } from "tslib"; - import __NG_CLI_RESOURCE__0 from "!${DirectAngularResourceLoaderPath}!./app.component.html"; - import __NG_CLI_RESOURCE__1 from "./app.component.css"; - import __NG_CLI_RESOURCE__2 from "./app.component.2.css"; + import __NG_CLI_RESOURCE__0 from "./app.component.html?ngResource"; + import __NG_CLI_RESOURCE__1 from "./app.component.css?ngResource"; + import __NG_CLI_RESOURCE__2 from "./app.component.2.css?ngResource"; import { Component as NgComponent } from '@angular/core'; let AppComponent = class AppComponent { @@ -301,7 +254,7 @@ describe('@ngtools/webpack transformers', () => { const { program } = createTypescriptContext(input); const getTypeChecker = () => program.getTypeChecker(); - const transformer = replaceResources(() => true, getTypeChecker, true); + const transformer = replaceResources(() => true, getTypeChecker); const result = transformTypescript(input, [transformer]); expect(tags.oneLine`${result}`).toEqual(tags.oneLine`${output}`); @@ -322,9 +275,9 @@ describe('@ngtools/webpack transformers', () => { `; const output = tags.stripIndent` import { __decorate } from "tslib"; - import __NG_CLI_RESOURCE__0 from "!${DirectAngularResourceLoaderPath}!./app.component.html"; - import __NG_CLI_RESOURCE__1 from "./app.component.css"; - import __NG_CLI_RESOURCE__2 from "./app.component.2.css"; + import __NG_CLI_RESOURCE__0 from "./app.component.html?ngResource"; + import __NG_CLI_RESOURCE__1 from "./app.component.css?ngResource"; + import __NG_CLI_RESOURCE__2 from "./app.component.2.css?ngResource"; import * as ng from '@angular/core'; let AppComponent = class AppComponent { @@ -367,8 +320,8 @@ describe('@ngtools/webpack transformers', () => { const output = tags.stripIndent` import { __decorate } from "tslib"; - import __NG_CLI_RESOURCE__0 from "!${DirectAngularResourceLoaderPath}!./app.component.html"; - import __NG_CLI_RESOURCE__1 from "./app.component.css"; + import __NG_CLI_RESOURCE__0 from "./app.component.html?ngResource"; + import __NG_CLI_RESOURCE__1 from "./app.component.css?ngResource"; import { Component } from '@angular/core'; From 4c1049e1a144f3908e67ae4c6d71ab890a70b492 Mon Sep 17 00:00:00 2001 From: Alan Agius Date: Thu, 2 Dec 2021 17:42:35 +0100 Subject: [PATCH 2/2] fix(@angular-devkit/build-angular): differentiate components and global styles using file query instead of filename Previously, we introduced the `ngResource` query to Angular component resources we now use it with `resourceQuery` to differentiate between global and components styles, since in some cases while unlikely a file can be used as a component and global style. Closes #7245 --- .../src/builders/browser/specs/styles_spec.ts | 22 +++++++++++++++++++ .../tests/options/named-chunks_spec.ts | 2 +- .../src/webpack/configs/styles.ts | 4 ++-- tests/legacy-cli/e2e/tests/build/worker.ts | 15 ++++--------- 4 files changed, 29 insertions(+), 14 deletions(-) diff --git a/packages/angular_devkit/build_angular/src/builders/browser/specs/styles_spec.ts b/packages/angular_devkit/build_angular/src/builders/browser/specs/styles_spec.ts index 867faac7fce8..71a39f0f3e5c 100644 --- a/packages/angular_devkit/build_angular/src/builders/browser/specs/styles_spec.ts +++ b/packages/angular_devkit/build_angular/src/builders/browser/specs/styles_spec.ts @@ -655,4 +655,26 @@ describe('Browser Builder styles', () => { result = await browserBuild(architect, host, target, { optimization: true }); expect(await result.files['styles.css']).toContain('rgba(0,0,0,.15)'); }); + + it('works when using the same css file in `styles` and `stylesUrl`', async () => { + host.writeMultipleFiles({ + 'src/styles.css': ` + div { color: red } + `, + './src/app/app.component.ts': ` + import { Component } from '@angular/core'; + + @Component({ + selector: 'app-root', + templateUrl: './app.component.html', + styleUrls: ['../styles.css'] + }) + export class AppComponent { + title = 'app'; + } + `, + }); + + await browserBuild(architect, host, target, { styles: ['src/styles.css'] }); + }); }); diff --git a/packages/angular_devkit/build_angular/src/builders/browser/tests/options/named-chunks_spec.ts b/packages/angular_devkit/build_angular/src/builders/browser/tests/options/named-chunks_spec.ts index 78e506bd1c24..87d67ac17461 100644 --- a/packages/angular_devkit/build_angular/src/builders/browser/tests/options/named-chunks_spec.ts +++ b/packages/angular_devkit/build_angular/src/builders/browser/tests/options/named-chunks_spec.ts @@ -11,7 +11,7 @@ import { BASE_OPTIONS, BROWSER_BUILDER_INFO, describeBuilder } from '../setup'; const MAIN_OUTPUT = 'dist/main.js'; const NAMED_LAZY_OUTPUT = 'dist/src_lazy-module_ts.js'; -const UNNAMED_LAZY_OUTPUT = 'dist/8.js'; +const UNNAMED_LAZY_OUTPUT = 'dist/459.js'; describeBuilder(buildWebpackBrowser, BROWSER_BUILDER_INFO, (harness) => { describe('Option: "namedChunks"', () => { diff --git a/packages/angular_devkit/build_angular/src/webpack/configs/styles.ts b/packages/angular_devkit/build_angular/src/webpack/configs/styles.ts index 6178826149cf..740999065244 100644 --- a/packages/angular_devkit/build_angular/src/webpack/configs/styles.ts +++ b/packages/angular_devkit/build_angular/src/webpack/configs/styles.ts @@ -377,14 +377,14 @@ export function getStylesConfig(wco: WebpackConfigOptions): Configuration { oneOf: [ // Component styles are all styles except defined global styles { - exclude: globalStylePaths, use: componentStyleLoaders, + resourceQuery: /\?ngResource/, type: 'asset/source', }, // Global styles are only defined global styles { - include: globalStylePaths, use: globalStyleLoaders, + resourceQuery: { not: [/\?ngResource/] }, }, ], }, diff --git a/tests/legacy-cli/e2e/tests/build/worker.ts b/tests/legacy-cli/e2e/tests/build/worker.ts index bd8c53440883..a930a8ce3ff2 100644 --- a/tests/legacy-cli/e2e/tests/build/worker.ts +++ b/tests/legacy-cli/e2e/tests/build/worker.ts @@ -6,19 +6,12 @@ * found in the LICENSE file at https://angular.io/license */ -import { join } from 'path'; -import { - appendToFile, - expectFileToExist, - expectFileToMatch, - replaceInFile, - writeFile, -} from '../../utils/fs'; +import { expectFileToExist, expectFileToMatch, replaceInFile, writeFile } from '../../utils/fs'; import { ng } from '../../utils/process'; export default async function () { - const workerPath = join('src', 'app', 'app.worker.ts'); - const snippetPath = join('src', 'app', 'app.component.ts'); + const workerPath = 'src/app/app.worker.ts'; + const snippetPath = 'src/app/app.component.ts'; const projectTsConfig = 'tsconfig.json'; const workerTsConfig = 'tsconfig.worker.json'; @@ -33,7 +26,7 @@ export default async function () { await expectFileToMatch('dist/test-project/main.js', 'src_app_app_worker_ts'); await ng('build', '--output-hashing=none'); - const chunkId = '310'; + const chunkId = '151'; await expectFileToExist(`dist/test-project/${chunkId}.js`); await expectFileToMatch('dist/test-project/main.js', chunkId);