From 6207d6f1f0771ff3b74379367e65af665ef0e51c Mon Sep 17 00:00:00 2001 From: Kristiyan Kostadinov Date: Wed, 1 Mar 2023 11:29:34 +0100 Subject: [PATCH 01/32] fix(migrations): add protractor support if protractor imports are detected (#49274) The new `bootstrapApplication` API doesn't include Protractor support anymore which may cause existing e2e tests to break after the migration. These changes add some logic that will provide Protractor support if any imports to `protractor` or `protractor/*` are detected. PR Close #49274 --- .../standalone-bootstrap.ts | 47 +++++++++- .../test/standalone_migration_spec.ts | 90 +++++++++++++++++++ 2 files changed, 134 insertions(+), 3 deletions(-) diff --git a/packages/core/schematics/ng-generate/standalone-migration/standalone-bootstrap.ts b/packages/core/schematics/ng-generate/standalone-migration/standalone-bootstrap.ts index 7b250ddd7d2c..0a8454596621 100644 --- a/packages/core/schematics/ng-generate/standalone-migration/standalone-bootstrap.ts +++ b/packages/core/schematics/ng-generate/standalone-migration/standalone-bootstrap.ts @@ -8,7 +8,7 @@ import {NgtscProgram} from '@angular/compiler-cli'; -import {Reference, TemplateTypeChecker} from '@angular/compiler-cli/private/migrations'; +import {TemplateTypeChecker} from '@angular/compiler-cli/private/migrations'; import {dirname, join} from 'path'; import ts from 'typescript'; @@ -45,6 +45,12 @@ export function toStandaloneBootstrap( const testObjects = new Set(); const allDeclarations = new Set(); + // `bootstrapApplication` doesn't include Protractor support by default + // anymore so we have to opt the app in, if we detect it being used. + const additionalProviders = hasImport(program, rootFileNames, 'protractor') ? + new Map([['provideProtractorTestingSupport', '@angular/platform-browser']]) : + null; + for (const sourceFile of sourceFiles) { sourceFile.forEachChild(function walk(node) { if (ts.isCallExpression(node) && ts.isPropertyAccessExpression(node.expression) && @@ -64,7 +70,8 @@ export function toStandaloneBootstrap( for (const call of bootstrapCalls) { call.declarations.forEach(decl => allDeclarations.add(decl)); - migrateBootstrapCall(call, tracker, referenceResolver, typeChecker, printer); + migrateBootstrapCall( + call, tracker, additionalProviders, referenceResolver, typeChecker, printer); } // The previous migrations explicitly skip over bootstrapped @@ -135,12 +142,15 @@ function analyzeBootstrapCall( * Converts a `bootstrapModule` call to `bootstrapApplication`. * @param analysis Analysis result of the call. * @param tracker Tracker in which to register the changes. + * @param additionalFeatures Additional providers, apart from the auto-detected ones, that should + * be added to the bootstrap call. * @param referenceResolver * @param typeChecker * @param printer */ function migrateBootstrapCall( - analysis: BootstrapCallAnalysis, tracker: ChangeTracker, referenceResolver: ReferenceResolver, + analysis: BootstrapCallAnalysis, tracker: ChangeTracker, + additionalProviders: Map|null, referenceResolver: ReferenceResolver, typeChecker: ts.TypeChecker, printer: ts.Printer) { const sourceFile = analysis.call.getSourceFile(); const moduleSourceFile = analysis.metadata.getSourceFile(); @@ -177,6 +187,13 @@ function migrateBootstrapCall( nodesToCopy, referenceResolver, typeChecker); } + if (additionalProviders) { + additionalProviders.forEach((moduleSpecifier, name) => { + providersInNewCall.push(ts.factory.createCallExpression( + tracker.addImport(sourceFile, name, moduleSpecifier), undefined, undefined)); + }); + } + if (nodesToCopy.size > 0) { let text = '\n\n'; nodesToCopy.forEach(node => { @@ -703,3 +720,27 @@ function getLastImportEnd(sourceFile: ts.SourceFile): number { return index; } + +/** Checks if any of the program's files has an import of a specific module. */ +function hasImport(program: NgtscProgram, rootFileNames: string[], moduleName: string): boolean { + const tsProgram = program.getTsProgram(); + const deepImportStart = moduleName + '/'; + + for (const fileName of rootFileNames) { + const sourceFile = tsProgram.getSourceFile(fileName); + + if (!sourceFile) { + continue; + } + + for (const statement of sourceFile.statements) { + if (ts.isImportDeclaration(statement) && ts.isStringLiteralLike(statement.moduleSpecifier) && + (statement.moduleSpecifier.text === moduleName || + statement.moduleSpecifier.text.startsWith(deepImportStart))) { + return true; + } + } + } + + return false; +} diff --git a/packages/core/schematics/test/standalone_migration_spec.ts b/packages/core/schematics/test/standalone_migration_spec.ts index f01447e9bd6f..1421c7d6f287 100644 --- a/packages/core/schematics/test/standalone_migration_spec.ts +++ b/packages/core/schematics/test/standalone_migration_spec.ts @@ -3647,4 +3647,94 @@ describe('standalone migration', () => { }).catch(e => console.error(e)); `)); }); + + it('should add Protractor support if any tests are detected', async () => { + writeFile('main.ts', ` + import {AppModule} from './app/app.module'; + import {platformBrowser} from '@angular/platform-browser'; + + platformBrowser().bootstrapModule(AppModule).catch(e => console.error(e)); + `); + + writeFile('./app/app.module.ts', ` + import {NgModule, Component} from '@angular/core'; + + @Component({selector: 'app', template: 'hello'}) + export class AppComponent {} + + @NgModule({declarations: [AppComponent], bootstrap: [AppComponent]}) + export class AppModule {} + `); + + writeFile('./app/app.e2e.spec.ts', ` + import {browser, by, element} from 'protractor'; + + describe('app', () => { + beforeAll(async () => { + await browser.get(browser.params.testUrl); + }); + + it('should work', async () => { + const rootSelector = element(by.css('app')); + expect(await rootSelector.isPresent()).toBe(true); + }); + }); + `); + + await runMigration('standalone-bootstrap'); + + expect(stripWhitespace(tree.readContent('main.ts'))).toBe(stripWhitespace(` + import {AppComponent} from './app/app.module'; + import {platformBrowser, provideProtractorTestingSupport, bootstrapApplication} from '@angular/platform-browser'; + + bootstrapApplication(AppComponent, { + providers: [provideProtractorTestingSupport()] + }).catch(e => console.error(e)); + `)); + }); + + it('should add Protractor support if any tests with deep imports are detected', async () => { + writeFile('main.ts', ` + import {AppModule} from './app/app.module'; + import {platformBrowser} from '@angular/platform-browser'; + + platformBrowser().bootstrapModule(AppModule).catch(e => console.error(e)); + `); + + writeFile('./app/app.module.ts', ` + import {NgModule, Component} from '@angular/core'; + + @Component({selector: 'app', template: 'hello'}) + export class AppComponent {} + + @NgModule({declarations: [AppComponent], bootstrap: [AppComponent]}) + export class AppModule {} + `); + + writeFile('./app/app.e2e.spec.ts', ` + import {browser, by, element} from 'protractor/some/deep-import'; + + describe('app', () => { + beforeAll(async () => { + await browser.get(browser.params.testUrl); + }); + + it('should work', async () => { + const rootSelector = element(by.css('app')); + expect(await rootSelector.isPresent()).toBe(true); + }); + }); + `); + + await runMigration('standalone-bootstrap'); + + expect(stripWhitespace(tree.readContent('main.ts'))).toBe(stripWhitespace(` + import {AppComponent} from './app/app.module'; + import {platformBrowser, provideProtractorTestingSupport, bootstrapApplication} from '@angular/platform-browser'; + + bootstrapApplication(AppComponent, { + providers: [provideProtractorTestingSupport()] + }).catch(e => console.error(e)); + `)); + }); }); From 819b9f32b4428533500a6536ec3d931178a8cb07 Mon Sep 17 00:00:00 2001 From: Andrew Kushnir Date: Tue, 28 Feb 2023 20:58:20 -0800 Subject: [PATCH 02/32] refactor(core): move `APP_BOOTSTRAP_LISTENER` to avoid circular deps (#49273) This commit moves the `APP_BOOTSTRAP_LISTENER` token into the `application_ref.ts` to avoid a risk of circular dependencies. The main problem is that the token refers to the `ComponentRef`, which in turn refers to more symbols, thus making the `application_tokens.ts` file susceptible to circular dependencies. Such a dependency was identified in https://github.com/angular/angular/pull/49271. PR Close #49273 --- packages/core/src/application_ref.ts | 17 +++++++++++++++-- packages/core/src/application_tokens.ts | 14 -------------- packages/core/src/core.ts | 4 ++-- 3 files changed, 17 insertions(+), 18 deletions(-) diff --git a/packages/core/src/application_ref.ts b/packages/core/src/application_ref.ts index 0a42b6ce816a..4c5d317f0f48 100644 --- a/packages/core/src/application_ref.ts +++ b/packages/core/src/application_ref.ts @@ -12,7 +12,7 @@ import {merge, Observable, Observer, Subscription} from 'rxjs'; import {share} from 'rxjs/operators'; import {ApplicationInitStatus} from './application_init'; -import {APP_BOOTSTRAP_LISTENER, PLATFORM_INITIALIZER} from './application_tokens'; +import {PLATFORM_INITIALIZER} from './application_tokens'; import {getCompilerFacade, JitCompilerUsage} from './compiler/compiler_facade'; import {Console} from './console'; import {Injectable} from './di/injectable'; @@ -46,6 +46,8 @@ import {scheduleMicroTask} from './util/microtask'; import {stringify} from './util/stringify'; import {NgZone, NoopNgZone} from './zone/ng_zone'; +const NG_DEV_MODE = typeof ngDevMode === 'undefined' || ngDevMode; + let _platformInjector: Injector|null = null; /** @@ -63,7 +65,18 @@ export const ALLOW_MULTIPLE_PLATFORMS = new InjectionToken('AllowMultip const PLATFORM_DESTROY_LISTENERS = new InjectionToken>('PlatformDestroyListeners'); -const NG_DEV_MODE = typeof ngDevMode === 'undefined' || ngDevMode; +/** + * A [DI token](guide/glossary#di-token "DI token definition") that provides a set of callbacks to + * be called for every component that is bootstrapped. + * + * Each callback must take a `ComponentRef` instance and return nothing. + * + * `(componentRef: ComponentRef) => void` + * + * @publicApi + */ +export const APP_BOOTSTRAP_LISTENER = + new InjectionToken) => void>>('appBootstrapListener'); export function compileNgModuleFactory( injector: Injector, options: CompilerOptions, diff --git a/packages/core/src/application_tokens.ts b/packages/core/src/application_tokens.ts index 23f7012fabdb..013bf0541656 100644 --- a/packages/core/src/application_tokens.ts +++ b/packages/core/src/application_tokens.ts @@ -7,7 +7,6 @@ */ import {InjectionToken} from './di'; -import {ComponentRef} from './linker/component_factory'; /** @@ -59,19 +58,6 @@ export const PLATFORM_ID = new InjectionToken('Platform ID', { factory: () => 'unknown', // set a default platform name, when none set explicitly }); -/** - * A [DI token](guide/glossary#di-token "DI token definition") that provides a set of callbacks to - * be called for every component that is bootstrapped. - * - * Each callback must take a `ComponentRef` instance and return nothing. - * - * `(componentRef: ComponentRef) => void` - * - * @publicApi - */ -export const APP_BOOTSTRAP_LISTENER = - new InjectionToken) => void>>('appBootstrapListener'); - /** * A [DI token](guide/glossary#di-token "DI token definition") that indicates the root directory of * the application diff --git a/packages/core/src/core.ts b/packages/core/src/core.ts index ac1fe45cef8d..76a2d04b45ab 100644 --- a/packages/core/src/core.ts +++ b/packages/core/src/core.ts @@ -15,9 +15,9 @@ export * from './metadata'; export * from './version'; export {TypeDecorator} from './util/decorators'; export * from './di'; -export {createPlatform, assertPlatform, destroyPlatform, getPlatform, BootstrapOptions, PlatformRef, ApplicationRef, createPlatformFactory, NgProbeToken} from './application_ref'; +export {createPlatform, assertPlatform, destroyPlatform, getPlatform, BootstrapOptions, PlatformRef, ApplicationRef, createPlatformFactory, NgProbeToken, APP_BOOTSTRAP_LISTENER} from './application_ref'; export {enableProdMode, isDevMode} from './util/is_dev_mode'; -export {APP_ID, PACKAGE_ROOT_URL, PLATFORM_INITIALIZER, PLATFORM_ID, APP_BOOTSTRAP_LISTENER, ANIMATION_MODULE_TYPE} from './application_tokens'; +export {APP_ID, PACKAGE_ROOT_URL, PLATFORM_INITIALIZER, PLATFORM_ID, ANIMATION_MODULE_TYPE} from './application_tokens'; export {APP_INITIALIZER, ApplicationInitStatus} from './application_init'; export * from './zone'; export * from './render'; From bca3aed9e48da38c5cdba782ed952b31f4e767ef Mon Sep 17 00:00:00 2001 From: Alan Agius Date: Tue, 28 Feb 2023 15:21:26 +0000 Subject: [PATCH 03/32] build: remove core-js-bundle from dependencies (#49259) This is no longer needed as Angular is ever green. PR Close #49259 --- integration/cli-hello-world-ivy-i18n/package.json | 1 - integration/dynamic-compiler/package.json | 1 - integration/npm_package_archives.bzl | 1 - karma-js.conf.js | 1 - package.json | 1 - packages/elements/test/BUILD.bazel | 1 - packages/zone.js/karma-build-jasmine.es2015.conf.js | 6 ------ packages/zone.js/karma-build.conf.js | 1 - packages/zone.js/karma-dist-sauce-jasmine.es2015.conf.js | 7 ------- packages/zone.js/karma-dist.conf.js | 1 - packages/zone.js/karma-evergreen-dist.conf.js | 8 -------- packages/zone.js/test/webdriver/test.html | 1 - yarn.lock | 5 ----- 13 files changed, 35 deletions(-) diff --git a/integration/cli-hello-world-ivy-i18n/package.json b/integration/cli-hello-world-ivy-i18n/package.json index c2a428bfd110..0d25037aa995 100644 --- a/integration/cli-hello-world-ivy-i18n/package.json +++ b/integration/cli-hello-world-ivy-i18n/package.json @@ -24,7 +24,6 @@ "@angular/platform-browser": "file:../../dist/packages-dist/platform-browser", "@angular/platform-browser-dynamic": "file:../../dist/packages-dist/platform-browser-dynamic", "@angular/router": "file:../../dist/packages-dist/router", - "core-js": "file:../../node_modules/core-js", "rxjs": "file:../../node_modules/rxjs", "tslib": "file:../../node_modules/tslib", "zone.js": "file:../../dist/zone.js-dist/archive/zone.js.tgz" diff --git a/integration/dynamic-compiler/package.json b/integration/dynamic-compiler/package.json index e62192c77169..90dc008b9281 100644 --- a/integration/dynamic-compiler/package.json +++ b/integration/dynamic-compiler/package.json @@ -34,7 +34,6 @@ "@angular/platform-browser": "file:../../dist/packages-dist/platform-browser", "@angular/platform-browser-dynamic": "file:../../dist/packages-dist/platform-browser-dynamic", "@angular/platform-server": "file:../../dist/packages-dist/platform-server", - "core-js": "file:../../node_modules/core-js", "rxjs": "file:../../node_modules/rxjs", "zone.js": "file:../../dist/zone.js-dist/archive/zone.js.tgz" } diff --git a/integration/npm_package_archives.bzl b/integration/npm_package_archives.bzl index 94a6458a9700..9cec413494b7 100644 --- a/integration/npm_package_archives.bzl +++ b/integration/npm_package_archives.bzl @@ -14,7 +14,6 @@ NPM_PACKAGE_ARCHIVES = [ "@rollup/plugin-node-resolve", "@rollup/plugin-commonjs", "check-side-effects", - "core-js", "google-closure-compiler", "jasmine", "typescript", diff --git a/karma-js.conf.js b/karma-js.conf.js index 92b37ec21891..f2c6df06117b 100644 --- a/karma-js.conf.js +++ b/karma-js.conf.js @@ -35,7 +35,6 @@ module.exports = function(config) { {pattern: 'node_modules/angular-1.8/angular?(.min).js', included: false, watched: false}, {pattern: 'node_modules/angular-mocks-1.8/angular-mocks.js', included: false, watched: false}, - 'node_modules/core-js-bundle/index.js', 'node_modules/jasmine-ajax/lib/mock-ajax.js', // Static test assets. diff --git a/package.json b/package.json index 7bc1dfe5c9a5..77091b2b0b77 100644 --- a/package.json +++ b/package.json @@ -121,7 +121,6 @@ "chalk": "^4.1.0", "chokidar": "^3.5.1", "convert-source-map": "^1.5.1", - "core-js-bundle": "^3.10.2", "d3": "^7.0.0", "dependency-graph": "^0.11.0", "diff": "^5.0.0", diff --git a/packages/elements/test/BUILD.bazel b/packages/elements/test/BUILD.bazel index c9e36d84b3ea..db65bbf67b20 100644 --- a/packages/elements/test/BUILD.bazel +++ b/packages/elements/test/BUILD.bazel @@ -30,7 +30,6 @@ filegroup( testonly = True, # do not sort srcs = [ - "@npm//:node_modules/core-js-bundle/index.js", # Required for browsers that do not natively support Custom Elements. "@npm//:node_modules/@webcomponents/custom-elements/custom-elements.min.js", "@npm//:node_modules/reflect-metadata/Reflect.js", diff --git a/packages/zone.js/karma-build-jasmine.es2015.conf.js b/packages/zone.js/karma-build-jasmine.es2015.conf.js index 6ab875d89d27..16eedcd7ae3e 100644 --- a/packages/zone.js/karma-build-jasmine.es2015.conf.js +++ b/packages/zone.js/karma-build-jasmine.es2015.conf.js @@ -1,11 +1,5 @@ module.exports = function(config) { require('./karma-build-jasmine.conf.js')(config); - for (let i = 0; i < config.files.length; i++) { - if (config.files[i] === 'node_modules/core-js-bundle/index.js') { - config.files.splice(i, 1); - break; - } - } config.client.entrypoint = 'browser_es2015_entry_point'; }; diff --git a/packages/zone.js/karma-build.conf.js b/packages/zone.js/karma-build.conf.js index 6405bd4cd55f..943a610509fd 100644 --- a/packages/zone.js/karma-build.conf.js +++ b/packages/zone.js/karma-build.conf.js @@ -8,7 +8,6 @@ module.exports = function(config) { require('./karma-base.conf.js')(config); - config.files.push('node_modules/core-js-bundle/index.js'); config.files.push('build/test/browser-env-setup.js'); config.files.push('build/test/wtf_mock.js'); config.files.push('build/test/test_fake_polyfill.js'); diff --git a/packages/zone.js/karma-dist-sauce-jasmine.es2015.conf.js b/packages/zone.js/karma-dist-sauce-jasmine.es2015.conf.js index 874076ad58ed..8f78f82a9154 100644 --- a/packages/zone.js/karma-dist-sauce-jasmine.es2015.conf.js +++ b/packages/zone.js/karma-dist-sauce-jasmine.es2015.conf.js @@ -2,13 +2,6 @@ module.exports = function(config) { require('./karma-dist-jasmine.conf.js')(config); require('./sauce.es2015.conf')(config); - const files = config.files; - config.files = []; - for (let i = 0; i < files.length; i++) { - if (files[i] !== 'node_modules/core-js-bundle/index.js' || files[i] === 'build/test/main.js') { - config.files.push(files[i]); - } - } config.files.push('build/test/wtf_mock.js'); config.files.push('build/test/test_fake_polyfill.js'); config.files.push('build/test/custom_error.js'); diff --git a/packages/zone.js/karma-dist.conf.js b/packages/zone.js/karma-dist.conf.js index 94de5abc606b..8349c7d07498 100644 --- a/packages/zone.js/karma-dist.conf.js +++ b/packages/zone.js/karma-dist.conf.js @@ -8,7 +8,6 @@ module.exports = function(config) { require('./karma-base.conf.js')(config); - config.files.push('node_modules/core-js-bundle/index.js'); config.files.push('build/test/browser-env-setup.js'); config.files.push('build/test/wtf_mock.js'); config.files.push('build/test/test_fake_polyfill.js'); diff --git a/packages/zone.js/karma-evergreen-dist.conf.js b/packages/zone.js/karma-evergreen-dist.conf.js index 420399449565..d7b15b15bc31 100644 --- a/packages/zone.js/karma-evergreen-dist.conf.js +++ b/packages/zone.js/karma-evergreen-dist.conf.js @@ -8,14 +8,6 @@ module.exports = function(config) { require('./karma-base.conf.js')(config); - const files = config.files; - config.files = []; - for (let i = 0; i < files.length; i++) { - if (files[i] !== 'node_modules/core-js-bundle/index.js') { - config.files.push(files[i]); - } - } - config.files.push('build/test/browser-env-setup.js'); config.files.push('build/test/wtf_mock.js'); config.files.push('build/test/test_fake_polyfill.js'); diff --git a/packages/zone.js/test/webdriver/test.html b/packages/zone.js/test/webdriver/test.html index be600e790368..b619bae1fc7b 100644 --- a/packages/zone.js/test/webdriver/test.html +++ b/packages/zone.js/test/webdriver/test.html @@ -1,7 +1,6 @@ - diff --git a/yarn.lock b/yarn.lock index 6fd26532263d..87a6e3a88d4d 100644 --- a/yarn.lock +++ b/yarn.lock @@ -6974,11 +6974,6 @@ copy-webpack-plugin@11.0.0: schema-utils "^4.0.0" serialize-javascript "^6.0.0" -core-js-bundle@^3.10.2: - version "3.23.4" - resolved "https://registry.yarnpkg.com/core-js-bundle/-/core-js-bundle-3.23.4.tgz#fb4ea6b4d5a0ba3166da1c8936bc18b9328a292b" - integrity sha512-E5kP515mQPnHj0LL0Z4UyQ8VHrhugJaq3GOropUnR/fU3QA3Xxs3vRgt6CwNDpgHYiruDa0jqHwNHwk5FbcwFg== - core-js-compat@^3.25.1: version "3.25.3" resolved "https://registry.yarnpkg.com/core-js-compat/-/core-js-compat-3.25.3.tgz#d6a442a03f4eade4555d4e640e6a06151dd95d38" From 8f7fbddb5181a49c34138f5cfa217c5a7925d1d0 Mon Sep 17 00:00:00 2001 From: Matthieu Riegler Date: Tue, 14 Feb 2023 22:53:41 +0100 Subject: [PATCH 04/32] refactor: remove duplicate key from component metadata (#49065) `directiveMetadata()` already assigns the `standalone` property to the `R3ComponentMetadataFacade` there is no need to do it twice. PR Close #49065 --- packages/core/src/render3/jit/directive.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/core/src/render3/jit/directive.ts b/packages/core/src/render3/jit/directive.ts index 99cc7991be59..34f90917c6f9 100644 --- a/packages/core/src/render3/jit/directive.ts +++ b/packages/core/src/render3/jit/directive.ts @@ -125,7 +125,6 @@ export function compileComponent(type: Type, metadata: Component): void { encapsulation, interpolation: metadata.interpolation, viewProviders: metadata.viewProviders || null, - isStandalone: !!metadata.standalone, }; compilationDepth++; From ffc684381f24be198fa494b7f5e018d8f191eeb0 Mon Sep 17 00:00:00 2001 From: Matthieu Riegler Date: Thu, 2 Mar 2023 14:08:33 +0100 Subject: [PATCH 05/32] docs: Add NG8107 entry : Optional chain not nullable extended diagnostic (#49287) PR Close #49287 --- aio/content/extended-diagnostics/NG8107.md | 86 ++++++++++++++++++++++ aio/content/extended-diagnostics/index.md | 3 +- 2 files changed, 88 insertions(+), 1 deletion(-) create mode 100644 aio/content/extended-diagnostics/NG8107.md diff --git a/aio/content/extended-diagnostics/NG8107.md b/aio/content/extended-diagnostics/NG8107.md new file mode 100644 index 000000000000..5d1f706d8a63 --- /dev/null +++ b/aio/content/extended-diagnostics/NG8107.md @@ -0,0 +1,86 @@ +@name Optional chain not nullable + +@description + +This diagnostic detects when the left side of an optional chain operation (`.?`) does not include `null` or `undefined` in its type in Angular templates. + + + +import {Component} from '@angular/core'; + +@Component({ + template: `
{{ foo?.bar }}
`, + // … +}) +class MyComponent { + // `foo` is declared as an object which *cannot* be `null` or `undefined`. + foo: { bar: string} = { bar: 'bar'}; +} + + + +## What should I do instead? + +Update the template and declared type to be in sync. Double-check the type of the input and confirm whether it is actually expected to be nullable. + +If the input should be nullable, add `null` or `undefined` to its type to indicate this. + + + +import {Component} from '@angular/core'; + +@Component({ + // If `foo` is nullish, `bar` won't be evaluated and the express will return the nullish value (`null` or `undefined`). + template: `
{{ foo?.bar }}
`, + // … +}) +class MyComponent { + foo: { bar: string} | null = { bar: 'bar'}; +} + + + +If the input should not be nullable, delete the `?` operator. + + + +import {Component} from '@angular/core'; + +@Component({ + // Template always displays `bar` as `foo` is guaranteed to never be `null` or `undefined` + template: `
{{ foo.bar }}
`, + // … +}) +class MyComponent { + foo: { bar: string} = { bar: 'bar'}; +} + + + +## What if I can't avoid this? + +This diagnostic can be disabled by editing the project's `tsconfig.json` file: + + + +{ + "angularCompilerOptions": { + "extendedDiagnostics": { + "checks": { + "optionalChainNotNullable": "suppress" + } + } + } +} + + + +See [extended diagnostic configuration](extended-diagnostics#configuration) for more info. + + + + + + + +@reviewed 2023-03-02 diff --git a/aio/content/extended-diagnostics/index.md b/aio/content/extended-diagnostics/index.md index c8d9dbf6a32a..497f65530324 100644 --- a/aio/content/extended-diagnostics/index.md +++ b/aio/content/extended-diagnostics/index.md @@ -11,9 +11,10 @@ Currently, Angular supports the following extended diagnostics: * [NG8101 - `invalidBananaInBox`](extended-diagnostics/NG8101) * [NG8102 - `nullishCoalescingNotNullable`](extended-diagnostics/NG8102) * [NG8103 - `missingControlFlowDirective`](extended-diagnostics/NG8103) +* [NG8104 - `textAttributeNotBinding`](extended-diagnostics/NG8104) * [NG8105 - `missingNgForOfLet`](extended-diagnostics/NG8105) * [NG8106 - `suffixNotSupported`](extended-diagnostics/NG8106) -* [NG8104 - `textAttributeNotBinding`](extended-diagnostics/NG8104) +* [NG8107 - `optionalChainNotNullable`](extended-diagnostics/NG8107) ## Configuration From d42032a3805f55124b0741a15669b2c97f952663 Mon Sep 17 00:00:00 2001 From: Matthieu Riegler Date: Thu, 2 Mar 2023 19:46:40 +0100 Subject: [PATCH 06/32] refactor(core): Remove `isListLikeIterable` from private export. (#49297) PR #48433 removed the last external usage of `isListLikeIterable`. We can now remove it from the private exports. PR Close #49297 --- packages/core/src/core_private_export.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/core/src/core_private_export.ts b/packages/core/src/core_private_export.ts index 91c47f6a2faf..eb0026ac1c23 100644 --- a/packages/core/src/core_private_export.ts +++ b/packages/core/src/core_private_export.ts @@ -31,7 +31,6 @@ export {coerceToBoolean as ɵcoerceToBoolean} from './util/coercion'; export {devModeEqual as ɵdevModeEqual} from './util/comparison'; export {makeDecorator as ɵmakeDecorator} from './util/decorators'; export {global as ɵglobal} from './util/global'; -export {isListLikeIterable as ɵisListLikeIterable} from './util/iterable'; export {isObservable as ɵisObservable, isPromise as ɵisPromise, isSubscribable as ɵisSubscribable} from './util/lang'; export {stringify as ɵstringify} from './util/stringify'; export {NOT_FOUND_CHECK_ONLY_ELEMENT_INJECTOR as ɵNOT_FOUND_CHECK_ONLY_ELEMENT_INJECTOR} from './view/provider_flags'; From e20b6d98d96cebd0d50fd20a16c7f2e84c203975 Mon Sep 17 00:00:00 2001 From: Paul Gschwendtner Date: Thu, 2 Mar 2023 13:59:37 +0000 Subject: [PATCH 07/32] build: remove `ts-node` forking bug workaround (#49289) In the past, `ts-node` had a bug that prevented forking processes when used in combination with `--esm`. We contributed a fix upstream to `ts-node` to fix this, and this commit updates to the latest version so that we can simplify our `ng-dev` invocation. https://github.com/TypeStrong/ts-node/commit/32d07e2b2fcbaab97c11e71ee5fc3a79fc20c802 Fixes #46858 PR Close #49289 --- package.json | 5 ++--- yarn.lock | 15 ++++----------- 2 files changed, 6 insertions(+), 14 deletions(-) diff --git a/package.json b/package.json index 77091b2b0b77..6135ece2f6bc 100644 --- a/package.json +++ b/package.json @@ -24,7 +24,7 @@ "/ ": "", "postinstall": "node --preserve-symlinks --preserve-symlinks-main ./tools/postinstall-patches.js && patch-package --patch-dir tools/esm-interop/patches/npm", "prepare": "husky install", - "ng-dev": "cross-env TS_NODE_PROJECT=$PWD/.ng-dev/tsconfig.json TS_NODE_TRANSPILE_ONLY=1 node --no-warnings --loader ts-node/esm node_modules/@angular/ng-dev/bundles/cli.mjs", + "ng-dev": "ts-node --esm --project .ng-dev/tsconfig.json --transpile-only node_modules/@angular/ng-dev/bundles/cli.mjs", "build": "ts-node --esm --project scripts/tsconfig.json scripts/build/build-packages-dist.mts", "test": "bazelisk test", "test:ci": "bazelisk test -- //... -//devtools/... -//aio/...", @@ -185,7 +185,6 @@ "cldr": "7.3.0", "cldrjs": "0.5.5", "conventional-changelog": "^3.1.24", - "cross-env": "^7.0.3", "firebase-tools": "^11.0.0", "glob": "8.1.0", "gulp": "^4.0.2", @@ -198,7 +197,7 @@ "prettier": "^2.5.1", "sauce-connect": "https://saucelabs.com/downloads/sc-4.8.1-linux.tar.gz", "semver": "^7.3.5", - "ts-node": "^10.8.1", + "ts-node": "^10.9.1", "tsec": "0.2.6", "tslint-eslint-rules": "5.4.0", "tslint-no-toplevel-property-access": "0.0.2", diff --git a/yarn.lock b/yarn.lock index 87a6e3a88d4d..afe72e64ed81 100644 --- a/yarn.lock +++ b/yarn.lock @@ -7062,13 +7062,6 @@ cross-env@^5.1.3: dependencies: cross-spawn "^6.0.5" -cross-env@^7.0.3: - version "7.0.3" - resolved "https://registry.yarnpkg.com/cross-env/-/cross-env-7.0.3.tgz#865264b29677dc015ba8418918965dd232fc54cf" - integrity sha512-+/HKd6EgcQCJGh2PSjZuUitQBQynKor4wrFbRg4DtAgS1aWO+gU52xpH7M9ScGgXSYmAVS9bIJ8EzuaGw0oNAw== - dependencies: - cross-spawn "^7.0.1" - cross-spawn@^6.0.5: version "6.0.5" resolved "https://registry.yarnpkg.com/cross-spawn/-/cross-spawn-6.0.5.tgz#4a5ec7c64dfae22c3a14124dbacdee846d80cbc4" @@ -16172,10 +16165,10 @@ ts-graphviz@^1.5.0: resolved "https://registry.yarnpkg.com/ts-graphviz/-/ts-graphviz-1.5.2.tgz#e0e803abe9dad923fa413208720776e3084e518b" integrity sha512-/V4N6wIvE+mcQicUfqSNxzFUX4xNccpwi3fGafEUSqGBSZYp9HVP8RZdjTfxCQvTjavxqUrEDB5ucEqKPLgp+w== -ts-node@^10.8.1: - version "10.8.2" - resolved "https://registry.yarnpkg.com/ts-node/-/ts-node-10.8.2.tgz#3185b75228cef116bf82ffe8762594f54b2a23f2" - integrity sha512-LYdGnoGddf1D6v8REPtIH+5iq/gTDuZqv2/UJUU7tKjuEU8xVZorBM+buCGNjj+pGEud+sOoM4CX3/YzINpENA== +ts-node@^10.9.1: + version "10.9.1" + resolved "https://registry.yarnpkg.com/ts-node/-/ts-node-10.9.1.tgz#e73de9102958af9e1f0b168a6ff320e25adcff4b" + integrity sha512-NtVysVPkxxrwFGUUxGYhfux8k78pQB3JqYBXlLRZgdGUqTO5wU/UyHop5p70iEbGhB7q5KmiZiU0Y3KlJrScEw== dependencies: "@cspotcode/source-map-support" "^0.8.0" "@tsconfig/node10" "^1.0.7" From 81e32f428366c382ad2bd68311e174e9d418f6c8 Mon Sep 17 00:00:00 2001 From: Andrew Scott Date: Tue, 28 Feb 2023 14:42:06 -0800 Subject: [PATCH 08/32] docs(router): Update component testing to user `RouterTestingHarness` (#48553) This commit updates the documentation on testing the `Router` to use the `RouterTestingHarness` rather than stubs. The stubs described in the previous form of this document actually creates tests which are incapable of catching bugs related to the component's interaction with the `Router`. In addition, managing the stubs is more difficult than using the real `Router` classes. Stubbing something like the `RouterLink` is quite harmful because it neither tests the actual URL being created, nor the end result of the navigation. There have been serveral bug fixes in the Router over the years the would change the outcome of these but would not be caught by tests which create a stub. PR Close #48553 --- .../testing/src/app/app.component.spec.ts | 73 +++-- .../app/dashboard/dashboard.component.spec.ts | 110 +++---- .../src/app/dashboard/dashboard.component.ts | 22 +- .../app/hero/hero-detail.component.spec.ts | 273 ++++++------------ .../src/app/hero/hero-detail.service.ts | 23 +- .../testing/src/app/model/hero.service.ts | 65 ++--- .../src/testing/activated-route-stub.ts | 29 -- .../examples/testing/src/testing/index.ts | 12 +- .../src/testing/router-link-directive-stub.ts | 30 -- .../guide/testing-components-scenarios.md | 130 +-------- 10 files changed, 227 insertions(+), 540 deletions(-) delete mode 100644 aio/content/examples/testing/src/testing/activated-route-stub.ts delete mode 100644 aio/content/examples/testing/src/testing/router-link-directive-stub.ts diff --git a/aio/content/examples/testing/src/app/app.component.spec.ts b/aio/content/examples/testing/src/app/app.component.spec.ts index a9802da33c4c..102c5faeae96 100644 --- a/aio/content/examples/testing/src/app/app.component.spec.ts +++ b/aio/content/examples/testing/src/app/app.component.spec.ts @@ -1,11 +1,10 @@ // #docplaster -import { Component, DebugElement, NO_ERRORS_SCHEMA } from '@angular/core'; -import { ComponentFixture, TestBed, waitForAsync } from '@angular/core/testing'; -import { By } from '@angular/platform-browser'; +import {Component, DebugElement, NO_ERRORS_SCHEMA} from '@angular/core'; +import {ComponentFixture, fakeAsync, TestBed, tick, waitForAsync} from '@angular/core/testing'; +import {By} from '@angular/platform-browser'; +import {provideRouter, Router, RouterLink} from '@angular/router'; -import { RouterLinkDirectiveStub } from '../testing'; - -import { AppComponent } from './app.component'; +import {AppComponent} from './app.component'; // #docregion component-stubs @Component({selector: 'app-banner', template: ''}) @@ -29,10 +28,10 @@ describe('AppComponent & TestModule', () => { // #docregion testbed-stubs TestBed .configureTestingModule({ - declarations: [ - AppComponent, RouterLinkDirectiveStub, BannerStubComponent, RouterOutletStubComponent, - WelcomeStubComponent - ] + imports: [RouterLink], + providers: [provideRouter([])], + declarations: + [AppComponent, BannerStubComponent, RouterOutletStubComponent, WelcomeStubComponent] }) // #enddocregion testbed-stubs .compileComponents() @@ -55,8 +54,9 @@ describe('AppComponent & NO_ERRORS_SCHEMA', () => { // #enddocregion no-errors-schema BannerStubComponent, // #docregion no-errors-schema - RouterLinkDirectiveStub ], + providers: [provideRouter([])], + imports: [RouterLink], schemas: [NO_ERRORS_SCHEMA] }) // #enddocregion no-errors-schema, mixed-setup @@ -70,21 +70,25 @@ describe('AppComponent & NO_ERRORS_SCHEMA', () => { }); //////// Testing w/ real root module ////// -// Tricky because we are disabling the router and its configuration -// Better to use RouterTestingModule -import { AppModule } from './app.module'; -import { AppRoutingModule } from './app-routing.module'; +import {AppModule} from './app.module'; +import {AppRoutingModule} from './app-routing.module'; describe('AppComponent & AppModule', () => { beforeEach(waitForAsync(() => { TestBed - .configureTestingModule({imports: [AppModule]}) + .configureTestingModule({ + imports: [AppModule], + }) // Get rid of app's Router configuration otherwise many failures. // Doing so removes Router declarations; add the Router stubs .overrideModule(AppModule, { remove: {imports: [AppRoutingModule]}, - add: {declarations: [RouterLinkDirectiveStub, RouterOutletStubComponent]} + add: { + declarations: [RouterOutletStubComponent], + imports: [RouterLink], + providers: [provideRouter([])], + } }) .compileComponents() @@ -99,7 +103,7 @@ describe('AppComponent & AppModule', () => { }); function tests() { - let routerLinks: RouterLinkDirectiveStub[]; + let routerLinks: RouterLink[]; let linkDes: DebugElement[]; // #docregion test-setup @@ -107,11 +111,11 @@ function tests() { fixture.detectChanges(); // trigger initial data binding // find DebugElements with an attached RouterLinkStubDirective - linkDes = fixture.debugElement.queryAll(By.directive(RouterLinkDirectiveStub)); + linkDes = fixture.debugElement.queryAll(By.directive(RouterLink)); // get attached link directive instances // using each DebugElement's injector - routerLinks = linkDes.map(de => de.injector.get(RouterLinkDirectiveStub)); + routerLinks = linkDes.map(de => de.injector.get(RouterLink)); }); // #enddocregion test-setup @@ -121,26 +125,21 @@ function tests() { // #docregion tests it('can get RouterLinks from template', () => { - expect(routerLinks.length) - .withContext('should have 3 routerLinks') - .toBe(3); - expect(routerLinks[0].linkParams).toBe('/dashboard'); - expect(routerLinks[1].linkParams).toBe('/heroes'); - expect(routerLinks[2].linkParams).toBe('/about'); + expect(routerLinks.length).withContext('should have 3 routerLinks').toBe(3); + expect(routerLinks[0].href).toBe('/dashboard'); + expect(routerLinks[1].href).toBe('/heroes'); + expect(routerLinks[2].href).toBe('/about'); }); - it('can click Heroes link in template', () => { - const heroesLinkDe = linkDes[1]; // heroes link DebugElement - const heroesLink = routerLinks[1]; // heroes link directive - - expect(heroesLink.navigatedTo) - .withContext('should not have navigated yet') - .toBeNull(); + it('can click Heroes link in template', fakeAsync(() => { + const heroesLinkDe = linkDes[1]; // heroes link DebugElement - heroesLinkDe.triggerEventHandler('click'); - fixture.detectChanges(); + TestBed.inject(Router).resetConfig([{path: '**', children: []}]); + heroesLinkDe.triggerEventHandler('click', {button: 0}); + tick(); + fixture.detectChanges(); - expect(heroesLink.navigatedTo).toBe('/heroes'); - }); + expect(TestBed.inject(Router).url).toBe('/heroes'); + })); // #enddocregion tests } diff --git a/aio/content/examples/testing/src/app/dashboard/dashboard.component.spec.ts b/aio/content/examples/testing/src/app/dashboard/dashboard.component.spec.ts index 1a896d496f4c..4aa6bcacdeb4 100644 --- a/aio/content/examples/testing/src/app/dashboard/dashboard.component.spec.ts +++ b/aio/content/examples/testing/src/app/dashboard/dashboard.component.spec.ts @@ -1,20 +1,24 @@ -// #docplaster -import { ComponentFixture, TestBed, waitForAsync } from '@angular/core/testing'; - -import { addMatchers, asyncData, click } from '../../testing'; -import { HeroService } from '../model/hero.service'; -import { getTestHeroes } from '../model/testing/test-heroes'; - -import { By } from '@angular/platform-browser'; -import { Router } from '@angular/router'; - -import { DashboardComponent } from './dashboard.component'; -import { DashboardModule } from './dashboard.module'; +import {provideHttpClient} from '@angular/common/http'; +import {HttpTestingController, provideHttpClientTesting} from '@angular/common/http/testing'; +import {NO_ERRORS_SCHEMA} from '@angular/core'; +import {TestBed, waitForAsync} from '@angular/core/testing'; +import {By} from '@angular/platform-browser'; +import {NavigationEnd, provideRouter, Router} from '@angular/router'; +import {RouterTestingHarness} from '@angular/router/testing'; +import {firstValueFrom} from 'rxjs'; +import {filter} from 'rxjs/operators'; + +import {addMatchers, click} from '../../testing'; +import {HeroService} from '../model/hero.service'; +import {getTestHeroes} from '../model/testing/test-heroes'; + +import {DashboardComponent} from './dashboard.component'; +import {DashboardModule} from './dashboard.module'; beforeEach(addMatchers); let comp: DashboardComponent; -let fixture: ComponentFixture; +let harness: RouterTestingHarness; //////// Deep //////////////// @@ -29,15 +33,15 @@ describe('DashboardComponent (deep)', () => { function clickForDeep() { // get first
- const heroEl: HTMLElement = fixture.nativeElement.querySelector('.hero'); + const heroEl: HTMLElement = harness.routeNativeElement!.querySelector('.hero')!; click(heroEl); + return firstValueFrom( + TestBed.inject(Router).events.pipe(filter(e => e instanceof NavigationEnd))); } }); //////// Shallow //////////////// -import { NO_ERRORS_SCHEMA } from '@angular/core'; - describe('DashboardComponent (shallow)', () => { beforeEach(() => { TestBed.configureTestingModule( @@ -50,33 +54,32 @@ describe('DashboardComponent (shallow)', () => { function clickForShallow() { // get first DebugElement - const heroDe = fixture.debugElement.query(By.css('dashboard-hero')); + const heroDe = harness.routeDebugElement!.query(By.css('dashboard-hero')); heroDe.triggerEventHandler('selected', comp.heroes[0]); + return Promise.resolve(); } }); /** Add TestBed providers, compile, and create DashboardComponent */ function compileAndCreate() { beforeEach(waitForAsync(() => { - // #docregion router-spy - const routerSpy = jasmine.createSpyObj('Router', ['navigateByUrl']); - const heroServiceSpy = jasmine.createSpyObj('HeroService', ['getHeroes']); - + // #docregion router-harness TestBed .configureTestingModule({ providers: [ - {provide: HeroService, useValue: heroServiceSpy}, {provide: Router, useValue: routerSpy} + provideRouter([{path: '**', component: DashboardComponent}]), + provideHttpClient(), + provideHttpClientTesting(), + HeroService, ] }) - // #enddocregion router-spy .compileComponents() - .then(() => { - fixture = TestBed.createComponent(DashboardComponent); - comp = fixture.componentInstance; - - // getHeroes spy returns observable of test heroes - heroServiceSpy.getHeroes.and.returnValue(asyncData(getTestHeroes())); + .then(async () => { + harness = await RouterTestingHarness.create(); + comp = await harness.navigateByUrl('/', DashboardComponent); + TestBed.inject(HttpTestingController).expectOne('api/heroes').flush(getTestHeroes()); }); + // #enddocregion router-harness })); } @@ -84,61 +87,38 @@ function compileAndCreate() { * The (almost) same tests for both. * Only change: the way that the first hero is clicked */ -function tests(heroClick: () => void) { - - it('should NOT have heroes before ngOnInit', () => { - expect(comp.heroes.length) - .withContext('should not have heroes before ngOnInit') - .toBe(0); - }); - - it('should NOT have heroes immediately after ngOnInit', () => { - fixture.detectChanges(); // runs initial lifecycle hooks - - expect(comp.heroes.length) - .withContext('should not have heroes until service promise resolves') - .toBe(0); - }); - +function tests(heroClick: () => Promise) { describe('after get dashboard heroes', () => { let router: Router; - // Trigger component so it gets heroes and binds to them + // Trigger component so it gets heroes and binds to them beforeEach(waitForAsync(() => { - router = fixture.debugElement.injector.get(Router); - fixture.detectChanges(); // runs ngOnInit -> getHeroes - fixture.whenStable() // No need for the `lastPromise` hack! - .then(() => fixture.detectChanges()); // bind to heroes + router = TestBed.inject(Router); + harness.detectChanges(); // runs ngOnInit -> getHeroes })); it('should HAVE heroes', () => { expect(comp.heroes.length) - .withContext('should have heroes after service promise resolves') - .toBeGreaterThan(0); + .withContext('should have heroes after service promise resolves') + .toBeGreaterThan(0); }); it('should DISPLAY heroes', () => { // Find and examine the displayed heroes // Look for them in the DOM by css class - const heroes = fixture.nativeElement.querySelectorAll('dashboard-hero'); - expect(heroes.length) - .withContext('should display 4 heroes') - .toBe(4); + const heroes = harness.routeNativeElement!.querySelectorAll('dashboard-hero'); + expect(heroes.length).withContext('should display 4 heroes').toBe(4); }); // #docregion navigate-test - it('should tell ROUTER to navigate when hero clicked', () => { - heroClick(); // trigger click on first inner
- - // args passed to router.navigateByUrl() spy - const spy = router.navigateByUrl as jasmine.Spy; - const navArgs = spy.calls.first().args[0]; + it('should tell navigate when hero clicked', async () => { + await heroClick(); // trigger click on first inner
// expecting to navigate to id of the component's first hero const id = comp.heroes[0].id; - expect(navArgs) - .withContext('should nav to HeroDetail for first hero') - .toBe('/heroes/' + id); + expect(TestBed.inject(Router).url) + .withContext('should nav to HeroDetail for first hero') + .toEqual(`/heroes/${id}`); }); // #enddocregion navigate-test }); diff --git a/aio/content/examples/testing/src/app/dashboard/dashboard.component.ts b/aio/content/examples/testing/src/app/dashboard/dashboard.component.ts index 9a65b047891c..83512c628f9b 100644 --- a/aio/content/examples/testing/src/app/dashboard/dashboard.component.ts +++ b/aio/content/examples/testing/src/app/dashboard/dashboard.component.ts @@ -1,29 +1,24 @@ // #docregion -import { Component, OnInit } from '@angular/core'; -import { Router } from '@angular/router'; +import {Component, OnInit} from '@angular/core'; +import {Router} from '@angular/router'; -import { Hero } from '../model/hero'; -import { HeroService } from '../model/hero.service'; +import {Hero} from '../model/hero'; +import {HeroService} from '../model/hero.service'; @Component({ selector: 'app-dashboard', templateUrl: './dashboard.component.html', - styleUrls: [ './dashboard.component.css' ] + styleUrls: ['./dashboard.component.css'] }) export class DashboardComponent implements OnInit { - heroes: Hero[] = []; // #docregion ctor - constructor( - private router: Router, - private heroService: HeroService) { - } + constructor(private router: Router, private heroService: HeroService) {} // #enddocregion ctor ngOnInit() { - this.heroService.getHeroes() - .subscribe(heroes => this.heroes = heroes.slice(1, 5)); + this.heroService.getHeroes().subscribe(heroes => this.heroes = heroes.slice(1, 5)); } // #docregion goto-detail @@ -35,7 +30,6 @@ export class DashboardComponent implements OnInit { get title() { const cnt = this.heroes.length; - return cnt === 0 ? 'No Heroes' : - cnt === 1 ? 'Top Hero' : `Top ${cnt} Heroes`; + return cnt === 0 ? 'No Heroes' : cnt === 1 ? 'Top Hero' : `Top ${cnt} Heroes`; } } diff --git a/aio/content/examples/testing/src/app/hero/hero-detail.component.spec.ts b/aio/content/examples/testing/src/app/hero/hero-detail.component.spec.ts index 5ef0f102a63f..384be7065a53 100644 --- a/aio/content/examples/testing/src/app/hero/hero-detail.component.spec.ts +++ b/aio/content/examples/testing/src/app/hero/hero-detail.component.spec.ts @@ -1,27 +1,26 @@ // #docplaster -import { ComponentFixture, fakeAsync, inject, TestBed, tick, waitForAsync } from '@angular/core/testing'; -import { Router } from '@angular/router'; +import {provideHttpClient} from '@angular/common/http'; +import {HttpTestingController, provideHttpClientTesting} from '@angular/common/http/testing'; +import {fakeAsync, TestBed, tick} from '@angular/core/testing'; +import {provideRouter, Router} from '@angular/router'; +import {RouterTestingHarness} from '@angular/router/testing'; -import { - ActivatedRoute, ActivatedRouteStub, asyncData, click -} from '../../testing'; +import {asyncData, click} from '../../testing'; +import {Hero} from '../model/hero'; +import {SharedModule} from '../shared/shared.module'; -import { Hero } from '../model/hero'; -import { HeroDetailComponent } from './hero-detail.component'; -import { HeroDetailService } from './hero-detail.service'; -import { HeroModule } from './hero.module'; +import {HeroDetailComponent} from './hero-detail.component'; +import {HeroDetailService} from './hero-detail.service'; +import {HeroListComponent} from './hero-list.component'; +import {HeroModule} from './hero.module'; ////// Testing Vars ////// -let activatedRoute: ActivatedRouteStub; let component: HeroDetailComponent; -let fixture: ComponentFixture; +let harness: RouterTestingHarness; let page: Page; ////// Tests ////// describe('HeroDetailComponent', () => { - beforeEach(() => { - activatedRoute = new ActivatedRouteStub(); - }); describe('with HeroModule setup', heroModuleSetup); describe('when override its provided HeroDetailService', overrideSetup); describe('with FormsModule setup', formsModuleSetup); @@ -30,10 +29,11 @@ describe('HeroDetailComponent', () => { /////////////////// +const testHero = getTestHeroes()[0]; function overrideSetup() { // #docregion hds-spy class HeroDetailServiceSpy { - testHero: Hero = {id: 42, name: 'Test Hero'}; + testHero: Hero = {...testHero}; /* emit cloned test hero */ getHero = jasmine.createSpy('getHero').and.callFake( @@ -46,33 +46,22 @@ function overrideSetup() { // #enddocregion hds-spy - // the `id` value is irrelevant because ignored by service stub - beforeEach(() => activatedRoute.setParamMap({id: 99999})); - // #docregion setup-override beforeEach(async () => { - const routerSpy = createRouterSpy(); - await TestBed .configureTestingModule({ imports: [HeroModule], providers: [ - {provide: ActivatedRoute, useValue: activatedRoute}, - {provide: Router, useValue: routerSpy}, - // #enddocregion setup-override + provideRouter([{path: 'heroes/:id', component: HeroDetailComponent}]), // HeroDetailService at this level is IRRELEVANT! {provide: HeroDetailService, useValue: {}} - // #docregion setup-override ] }) - - // Override component's own provider // #docregion override-component-method .overrideComponent( HeroDetailComponent, {set: {providers: [{provide: HeroDetailService, useClass: HeroDetailServiceSpy}]}}) // #enddocregion override-component-method - .compileComponents(); }); // #enddocregion setup-override @@ -81,18 +70,22 @@ function overrideSetup() { let hdsSpy: HeroDetailServiceSpy; beforeEach(async () => { - await createComponent(); + harness = await RouterTestingHarness.create(); + component = await harness.navigateByUrl(`/heroes/${testHero.id}`, HeroDetailComponent); + page = new Page(); // get the component's injected HeroDetailServiceSpy - hdsSpy = fixture.debugElement.injector.get(HeroDetailService) as any; + hdsSpy = harness.routeDebugElement!.injector.get(HeroDetailService) as any; + + harness.detectChanges(); }); it('should have called `getHero`', () => { expect(hdsSpy.getHero.calls.count()) - .withContext('getHero called once') - .toBe(1, 'getHero called once'); + .withContext('getHero called once') + .toBe(1, 'getHero called once'); }); - it("should display stub hero's name", () => { + it('should display stub hero\'s name', () => { expect(page.nameDisplay.textContent).toBe(hdsSpy.testHero.name); }); @@ -102,62 +95,43 @@ function overrideSetup() { page.nameInput.value = newName; - page.nameInput.dispatchEvent(new Event('input')); // tell Angular + page.nameInput.dispatchEvent(new Event('input')); // tell Angular - expect(component.hero.name) - .withContext('component hero has new name') - .toBe(newName); + expect(component.hero.name).withContext('component hero has new name').toBe(newName); expect(hdsSpy.testHero.name) - .withContext('service hero unchanged before save') - .toBe(origName); + .withContext('service hero unchanged before save') + .toBe(origName); click(page.saveBtn); - expect(hdsSpy.saveHero.calls.count()) - .withContext('saveHero called once') - .toBe(1); + expect(hdsSpy.saveHero.calls.count()).withContext('saveHero called once').toBe(1); tick(); // wait for async save to complete expect(hdsSpy.testHero.name) - .withContext('service hero has new name after save') - .toBe(newName); - expect(page.navigateSpy.calls.any()) - .withContext('router.navigate called') - .toBe(true); - })); - // #enddocregion override-tests - - it('fixture injected service is not the component injected service', - // inject gets the service from the fixture - inject([HeroDetailService], (fixtureService: HeroDetailService) => { - // use `fixture.debugElement.injector` to get service from component - const componentService = fixture.debugElement.injector.get(HeroDetailService); - - expect(fixtureService) - .withContext('service injected from fixture') - .not.toBe(componentService); + .withContext('service hero has new name after save') + .toBe(newName); + expect(TestBed.inject(Router).url).toEqual('/heroes'); })); } //////////////////// -import { getTestHeroes, TestHeroService, HeroService } from '../model/testing/test-hero.service'; +import {getTestHeroes} from '../model/testing/test-hero.service'; const firstHero = getTestHeroes()[0]; function heroModuleSetup() { // #docregion setup-hero-module beforeEach(async () => { - const routerSpy = createRouterSpy(); - await TestBed .configureTestingModule({ imports: [HeroModule], - // #enddocregion setup-hero-module // declarations: [ HeroDetailComponent ], // NO! DOUBLE DECLARATION - // #docregion setup-hero-module providers: [ - {provide: ActivatedRoute, useValue: activatedRoute}, - {provide: HeroService, useClass: TestHeroService}, - {provide: Router, useValue: routerSpy}, + provideRouter([ + {path: 'heroes/:id', component: HeroDetailComponent}, + {path: 'heroes', component: HeroListComponent}, + ]), + provideHttpClient(), + provideHttpClientTesting(), ] }) .compileComponents(); @@ -170,50 +144,35 @@ function heroModuleSetup() { beforeEach(async () => { expectedHero = firstHero; - activatedRoute.setParamMap({id: expectedHero.id}); - await createComponent(); + await createComponent(expectedHero.id); }); - // #docregion selected-tests - it("should display that hero's name", () => { + it('should display that hero\'s name', () => { expect(page.nameDisplay.textContent).toBe(expectedHero.name); }); // #enddocregion route-good-id it('should navigate when click cancel', () => { click(page.cancelBtn); - expect(page.navigateSpy.calls.any()) - .withContext('router.navigate called') - .toBe(true); + expect(TestBed.inject(Router).url).toEqual(`/heroes/${expectedHero.id}`); }); it('should save when click save but not navigate immediately', () => { - // Get service injected into component and spy on its`saveHero` method. - // It delegates to fake `HeroService.updateHero` which delivers a safe test result. - const hds = fixture.debugElement.injector.get(HeroDetailService); - const saveSpy = spyOn(hds, 'saveHero').and.callThrough(); - click(page.saveBtn); - expect(saveSpy.calls.any()) - .withContext('HeroDetailService.save called') - .toBe(true); - expect(page.navigateSpy.calls.any()) - .withContext('router.navigate not called') - .toBe(false); + expect(TestBed.inject(HttpTestingController).expectOne({method: 'PUT', url: 'api/heroes'})); + expect(TestBed.inject(Router).url).toEqual('/heroes/41'); }); it('should navigate when click save and save resolves', fakeAsync(() => { click(page.saveBtn); tick(); // wait for async save to complete - expect(page.navigateSpy.calls.any()) - .withContext('router.navigate called') - .toBe(true); + expect(TestBed.inject(Router).url).toEqual('/heroes/41'); })); // #docregion title-case-pipe it('should convert hero name to Title Case', () => { // get the name's input and display elements from the DOM - const hostElement: HTMLElement = fixture.nativeElement; + const hostElement: HTMLElement = harness.routeNativeElement!; const nameInput: HTMLInputElement = hostElement.querySelector('input')!; const nameDisplay: HTMLElement = hostElement.querySelector('span')!; @@ -224,146 +183,97 @@ function heroModuleSetup() { nameInput.dispatchEvent(new Event('input')); // Tell Angular to update the display binding through the title pipe - fixture.detectChanges(); + harness.detectChanges(); expect(nameDisplay.textContent).toBe('Quick Brown Fox'); }); - // #enddocregion title-case-pipe // #enddocregion selected-tests - // #docregion route-good-id - }); - // #enddocregion route-good-id - // #docregion route-no-id - describe('when navigate with no hero id', () => { - beforeEach(async () => { - await createComponent(); - }); - - it('should have hero.id === 0', () => { - expect(component.hero.id).toBe(0); - }); - - it('should display empty hero name', () => { - expect(page.nameDisplay.textContent).toBe(''); - }); + // #enddocregion title-case-pipe }); - // #enddocregion route-no-id // #docregion route-bad-id describe('when navigate to non-existent hero id', () => { beforeEach(async () => { - activatedRoute.setParamMap({id: 99999}); - await createComponent(); + await createComponent(999); }); it('should try to navigate back to hero list', () => { - expect(page.gotoListSpy.calls.any()) - .withContext('comp.gotoList called') - .toBe(true); - expect(page.navigateSpy.calls.any()) - .withContext('router.navigate called') - .toBe(true); + expect(TestBed.inject(Router).url).toEqual('/heroes'); }); }); // #enddocregion route-bad-id - - // Why we must use `fixture.debugElement.injector` in `Page()` - it("cannot use `inject` to get component's provided HeroDetailService", () => { - let service: HeroDetailService; - fixture = TestBed.createComponent(HeroDetailComponent); - expect( - // Throws because `inject` only has access to TestBed's injector - // which is an ancestor of the component's injector - inject([HeroDetailService], (hds: HeroDetailService) => service = hds)) - .toThrowError(/No provider for HeroDetailService/); - - // get `HeroDetailService` with component's own injector - service = fixture.debugElement.injector.get(HeroDetailService); - expect(service) - .withContext('debugElement.injector') - .toBeDefined(); - }); } ///////////////////// -import { FormsModule } from '@angular/forms'; -import { TitleCasePipe } from '../shared/title-case.pipe'; +import {FormsModule} from '@angular/forms'; +import {TitleCasePipe} from '../shared/title-case.pipe'; function formsModuleSetup() { // #docregion setup-forms-module beforeEach(async () => { - const routerSpy = createRouterSpy(); - await TestBed .configureTestingModule({ imports: [FormsModule], declarations: [HeroDetailComponent, TitleCasePipe], providers: [ - {provide: ActivatedRoute, useValue: activatedRoute}, - {provide: HeroService, useClass: TestHeroService}, - {provide: Router, useValue: routerSpy}, + provideHttpClient(), + provideHttpClientTesting(), + provideRouter([{path: 'heroes/:id', component: HeroDetailComponent}]), ] }) .compileComponents(); }); // #enddocregion setup-forms-module - it("should display 1st hero's name", waitForAsync(() => { - const expectedHero = firstHero; - activatedRoute.setParamMap({id: expectedHero.id}); - createComponent().then(() => { - expect(page.nameDisplay.textContent).toBe(expectedHero.name); - }); - })); + it('should display 1st hero\'s name', async () => { + const expectedHero = firstHero; + await createComponent(expectedHero.id).then(() => { + expect(page.nameDisplay.textContent).toBe(expectedHero.name); + }); + }); } /////////////////////// -import { SharedModule } from '../shared/shared.module'; function sharedModuleSetup() { // #docregion setup-shared-module beforeEach(async () => { - const routerSpy = createRouterSpy(); - await TestBed .configureTestingModule({ imports: [SharedModule], declarations: [HeroDetailComponent], providers: [ - {provide: ActivatedRoute, useValue: activatedRoute}, - {provide: HeroService, useClass: TestHeroService}, - {provide: Router, useValue: routerSpy}, + provideRouter([{path: 'heroes/:id', component: HeroDetailComponent}]), + provideHttpClient(), + provideHttpClientTesting(), ] }) .compileComponents(); }); // #enddocregion setup-shared-module - it("should display 1st hero's name", waitForAsync(() => { - const expectedHero = firstHero; - activatedRoute.setParamMap({id: expectedHero.id}); - createComponent().then(() => { - expect(page.nameDisplay.textContent).toBe(expectedHero.name); - }); - })); + it('should display 1st hero\'s name', async () => { + const expectedHero = firstHero; + await createComponent(expectedHero.id).then(() => { + expect(page.nameDisplay.textContent).toBe(expectedHero.name); + }); + }); } /////////// Helpers ///// -// #docregion create-component /** Create the HeroDetailComponent, initialize it, set test variables */ -function createComponent() { - fixture = TestBed.createComponent(HeroDetailComponent); - component = fixture.componentInstance; - page = new Page(fixture); - - // 1st change detection triggers ngOnInit which gets a hero - fixture.detectChanges(); - return fixture.whenStable().then(() => { - // 2nd change detection displays the async-fetched hero - fixture.detectChanges(); - }); +// #docregion create-component +async function createComponent(id: number) { + harness = await RouterTestingHarness.create(); + component = await harness.navigateByUrl(`/heroes/${id}`, HeroDetailComponent); + page = new Page(); + + const request = TestBed.inject(HttpTestingController).expectOne(`api/heroes/?id=${id}`); + const hero = getTestHeroes().find(h => h.id === Number(id)); + request.flush(hero ? [hero] : []); + harness.detectChanges(); } // #enddocregion create-component @@ -386,30 +296,13 @@ class Page { return this.query('input'); } - gotoListSpy: jasmine.Spy; - navigateSpy: jasmine.Spy; - - constructor(someFixture: ComponentFixture) { - // get the navigate spy from the injected router spy object - const routerSpy = someFixture.debugElement.injector.get(Router) as any; - this.navigateSpy = routerSpy.navigate; - - // spy on component's `gotoList()` method - const someComponent = someFixture.componentInstance; - this.gotoListSpy = spyOn(someComponent, 'gotoList').and.callThrough(); - } - //// query helpers //// private query(selector: string): T { - return fixture.nativeElement.querySelector(selector); + return harness.routeNativeElement!.querySelector(selector)! as T; } private queryAll(selector: string): T[] { - return fixture.nativeElement.querySelectorAll(selector); + return harness.routeNativeElement!.querySelectorAll(selector) as any as T[]; } } // #enddocregion page - -function createRouterSpy() { - return jasmine.createSpyObj('Router', ['navigate']); -} diff --git a/aio/content/examples/testing/src/app/hero/hero-detail.service.ts b/aio/content/examples/testing/src/app/hero/hero-detail.service.ts index 71befa59ccba..16abbc88c0ff 100644 --- a/aio/content/examples/testing/src/app/hero/hero-detail.service.ts +++ b/aio/content/examples/testing/src/app/hero/hero-detail.service.ts @@ -1,30 +1,29 @@ -import { Injectable } from '@angular/core'; +import {Injectable} from '@angular/core'; +import {Observable} from 'rxjs'; +import {map} from 'rxjs/operators'; -import { Observable } from 'rxjs'; -import { map } from 'rxjs/operators'; - -import { Hero } from '../model/hero'; -import { HeroService } from '../model/hero.service'; +import {Hero} from '../model/hero'; +import {HeroService} from '../model/hero.service'; // #docregion prototype -@Injectable() +@Injectable({providedIn: 'root'}) export class HeroDetailService { - constructor(private heroService: HeroService) { } -// #enddocregion prototype + constructor(private heroService: HeroService) {} + // #enddocregion prototype // Returns a clone which caller may modify safely - getHero(id: number | string): Observable { + getHero(id: number|string): Observable { if (typeof id === 'string') { id = parseInt(id, 10); } return this.heroService.getHero(id).pipe( - map(hero => hero ? Object.assign({}, hero) : null) // clone or null + map(hero => hero ? Object.assign({}, hero) : null) // clone or null ); } saveHero(hero: Hero) { return this.heroService.updateHero(hero); } -// #docregion prototype + // #docregion prototype } // #enddocregion prototype diff --git a/aio/content/examples/testing/src/app/model/hero.service.ts b/aio/content/examples/testing/src/app/model/hero.service.ts index 933ae1eee33a..6dbc9137fca9 100644 --- a/aio/content/examples/testing/src/app/model/hero.service.ts +++ b/aio/content/examples/testing/src/app/model/hero.service.ts @@ -1,74 +1,69 @@ -import { Injectable } from '@angular/core'; -import { HttpClient, HttpHeaders, HttpErrorResponse } from '@angular/common/http'; +import {HttpClient, HttpErrorResponse, HttpHeaders} from '@angular/common/http'; +import {Injectable} from '@angular/core'; +import {Observable} from 'rxjs'; +import {catchError, map, tap} from 'rxjs/operators'; -import { Observable } from 'rxjs'; -import { catchError, map, tap } from 'rxjs/operators'; - -import { Hero } from './hero'; +import {Hero} from './hero'; const httpOptions = { - headers: new HttpHeaders({ 'Content-Type': 'application/json' }) + headers: new HttpHeaders({'Content-Type': 'application/json'}) }; -@Injectable() +@Injectable({providedIn: 'root'}) export class HeroService { - readonly heroesUrl = 'api/heroes'; // URL to web api - constructor(private http: HttpClient) { } + constructor(private http: HttpClient) {} /** GET heroes from the server */ getHeroes(): Observable { return this.http.get(this.heroesUrl) - .pipe( - tap(heroes => this.log('fetched heroes')), - catchError(this.handleError('getHeroes')) - ) as Observable; + .pipe( + tap(heroes => this.log('fetched heroes')), + catchError(this.handleError('getHeroes'))) as Observable; } /** GET hero by id. Return `undefined` when id not found */ - getHero(id: number | string): Observable { + getHero(id: number|string): Observable { if (typeof id === 'string') { id = parseInt(id, 10); } const url = `${this.heroesUrl}/?id=${id}`; - return this.http.get(url) - .pipe( - map(heroes => heroes[0]), // returns a {0|1} element array + return this.http.get(url).pipe( + map(heroes => heroes[0]), // returns a {0|1} element array tap(h => { const outcome = h ? 'fetched' : 'did not find'; this.log(`${outcome} hero id=${id}`); }), - catchError(this.handleError(`getHero id=${id}`)) - ); + catchError(this.handleError(`getHero id=${id}`))); } //////// Save methods ////////// /** POST: add a new hero to the server */ addHero(hero: Hero): Observable { - return this.http.post(this.heroesUrl, hero, httpOptions).pipe( - tap((addedHero) => this.log(`added hero w/ id=${addedHero.id}`)), - catchError(this.handleError('addHero')) - ); + return this.http.post(this.heroesUrl, hero, httpOptions) + .pipe( + tap((addedHero) => this.log(`added hero w/ id=${addedHero.id}`)), + catchError(this.handleError('addHero'))); } /** DELETE: delete the hero from the server */ - deleteHero(hero: Hero | number): Observable { + deleteHero(hero: Hero|number): Observable { const id = typeof hero === 'number' ? hero : hero.id; const url = `${this.heroesUrl}/${id}`; - return this.http.delete(url, httpOptions).pipe( - tap(_ => this.log(`deleted hero id=${id}`)), - catchError(this.handleError('deleteHero')) - ); + return this.http.delete(url, httpOptions) + .pipe( + tap(_ => this.log(`deleted hero id=${id}`)), + catchError(this.handleError('deleteHero'))); } /** PUT: update the hero on the server */ updateHero(hero: Hero): Observable { - return this.http.put(this.heroesUrl, hero, httpOptions).pipe( - tap(_ => this.log(`updated hero id=${hero.id}`)), - catchError(this.handleError('updateHero')) - ); + return this.http.put(this.heroesUrl, hero, httpOptions) + .pipe( + tap(_ => this.log(`updated hero id=${hero.id}`)), + catchError(this.handleError('updateHero'))); } /** * Returns a function that handles Http operation failures. @@ -78,9 +73,8 @@ export class HeroService { */ private handleError(operation = 'operation') { return (error: HttpErrorResponse): Observable => { - // TODO: send the error to remote logging infrastructure - console.error(error); // log to console instead + console.error(error); // log to console instead // If a native error is caught, do not transform it. We only want to // transform response errors that are not wrapped in an `Error`. @@ -92,7 +86,6 @@ export class HeroService { // TODO: better job of transforming error for user consumption throw new Error(`${operation} failed: ${message}`); }; - } private log(message: string) { diff --git a/aio/content/examples/testing/src/testing/activated-route-stub.ts b/aio/content/examples/testing/src/testing/activated-route-stub.ts deleted file mode 100644 index 59fec15af781..000000000000 --- a/aio/content/examples/testing/src/testing/activated-route-stub.ts +++ /dev/null @@ -1,29 +0,0 @@ -// export for convenience. -export { ActivatedRoute } from '@angular/router'; - -// #docregion activated-route-stub -import { convertToParamMap, ParamMap, Params } from '@angular/router'; -import { ReplaySubject } from 'rxjs'; - -/** - * An ActivateRoute test double with a `paramMap` observable. - * Use the `setParamMap()` method to add the next `paramMap` value. - */ -export class ActivatedRouteStub { - // Use a ReplaySubject to share previous values with subscribers - // and pump new values into the `paramMap` observable - private subject = new ReplaySubject(); - - constructor(initialParams?: Params) { - this.setParamMap(initialParams); - } - - /** The mock paramMap observable */ - readonly paramMap = this.subject.asObservable(); - - /** Set the paramMap observable's next value */ - setParamMap(params: Params = {}) { - this.subject.next(convertToParamMap(params)); - } -} -// #enddocregion activated-route-stub diff --git a/aio/content/examples/testing/src/testing/index.ts b/aio/content/examples/testing/src/testing/index.ts index bcea4482a219..3c930d4a3739 100644 --- a/aio/content/examples/testing/src/testing/index.ts +++ b/aio/content/examples/testing/src/testing/index.ts @@ -1,10 +1,8 @@ -import { DebugElement } from '@angular/core'; -import { tick, ComponentFixture } from '@angular/core/testing'; +import {DebugElement} from '@angular/core'; +import {ComponentFixture, tick} from '@angular/core/testing'; export * from './async-observable-helpers'; -export * from './activated-route-stub'; export * from './jasmine-matchers'; -export * from './router-link-directive-stub'; ///// Short utilities ///// @@ -18,12 +16,12 @@ export function advance(f: ComponentFixture): void { // #docregion click-event /** Button events to pass to `DebugElement.triggerEventHandler` for RouterLink event handler */ export const ButtonClickEvents = { - left: { button: 0 }, - right: { button: 2 } + left: {button: 0}, + right: {button: 2} }; /** Simulate element click. Defaults to mouse left-button click event. */ -export function click(el: DebugElement | HTMLElement, eventObj: any = ButtonClickEvents.left): void { +export function click(el: DebugElement|HTMLElement, eventObj: any = ButtonClickEvents.left): void { if (el instanceof HTMLElement) { el.click(); } else { diff --git a/aio/content/examples/testing/src/testing/router-link-directive-stub.ts b/aio/content/examples/testing/src/testing/router-link-directive-stub.ts deleted file mode 100644 index e1967ab3bd66..000000000000 --- a/aio/content/examples/testing/src/testing/router-link-directive-stub.ts +++ /dev/null @@ -1,30 +0,0 @@ -import { Directive, Input, HostListener } from '@angular/core'; - -// export for convenience. -export { RouterLink} from '@angular/router'; - -/* eslint-disable @angular-eslint/directive-class-suffix, @angular-eslint/directive-selector */ -// #docregion router-link -@Directive({ - selector: '[routerLink]' -}) -export class RouterLinkDirectiveStub { - @Input('routerLink') linkParams: any; - navigatedTo: any = null; - - @HostListener('click') - onClick() { - this.navigatedTo = this.linkParams; - } -} -// #enddocregion router-link - -/// Dummy module to satisfy Angular Language service. Never used. -import { NgModule } from '@angular/core'; - -@NgModule({ - declarations: [ - RouterLinkDirectiveStub - ] -}) -export class RouterStubsModule {} diff --git a/aio/content/guide/testing-components-scenarios.md b/aio/content/guide/testing-components-scenarios.md index 0d6b99229941..192398a9fe13 100644 --- a/aio/content/guide/testing-components-scenarios.md +++ b/aio/content/guide/testing-components-scenarios.md @@ -812,22 +812,13 @@ Testing the `DashboardComponent` seemed daunting in part because it involves the -Mocking the `HeroService` with a spy is a [familiar story](#component-with-async-service). -But the `Router` has a complicated API and is entwined with other services and application preconditions. -Might it be difficult to mock? - -Fortunately, not in this case because the `DashboardComponent` isn't doing much with the `Router` - -This is often the case with *routing components*. -As a rule you test the component, not the router, and care only if the component navigates with the right address under the given conditions. +Angular provides test helpers to reduce boilerplate and more effectively test code which depends on the Router and HttpClient. -Providing a router spy for *this component* test suite happens to be as easy as providing a `HeroService` spy. + - - -The following test clicks the displayed hero and confirms that `Router.navigateByUrl` is called with the expected url. +The following test clicks the displayed hero and confirms that we navigate to the expected URL. @@ -863,36 +854,9 @@ The [ActivatedRoute in action](guide/router-tutorial-toh#activated-route-in-acti
-Tests can explore how the `HeroDetailComponent` responds to different `id` parameter values by manipulating the `ActivatedRoute` injected into the component's constructor. - -You know how to spy on the `Router` and a data service. - -You'll take a different approach with `ActivatedRoute` because - -* `paramMap` returns an `Observable` that can emit more than one value during a test -* You need the router helper function, `convertToParamMap()`, to create a `ParamMap` -* Other *routed component* tests need a test double for `ActivatedRoute` - -These differences argue for a re-usable stub class. - -#### `ActivatedRouteStub` - -The following `ActivatedRouteStub` class serves as a test double for `ActivatedRoute`. - - - -Consider placing such helpers in a `testing` folder sibling to the `app` folder. -This sample puts `ActivatedRouteStub` in `testing/activated-route-stub.ts`. - -
- -Consider writing a more capable version of this stub class with the [*marble testing library*](#marble-testing). - -
- - +Tests can explore how the `HeroDetailComponent` responds to different `id` parameter values by navigating to different routes. -#### Testing with `ActivatedRouteStub` +#### Testing with the `RouterTestingHarness` Here's a test demonstrating the component's behavior when the observed `id` refers to an existing hero: @@ -907,21 +871,12 @@ Rely on your intuition for now. When the `id` cannot be found, the component should re-route to the `HeroListComponent`. -The test suite setup provided the same router spy [described above](#routing-component) which spies on the router without actually navigating. +The test suite setup provided the same router harness [described above](#routing-component). This test expects the component to try to navigate to the `HeroListComponent`. -While this application doesn't have a route to the `HeroDetailComponent` that omits the `id` parameter, it might add such a route someday. -The component should do something reasonable when there is no `id`. - -In this implementation, the component should create and display a new hero. -New heroes have `id=0` and a blank `name`. -This test confirms that the component behaves as expected: - - - ## Nested component tests Component templates often have nested components, whose templates might contain more components. @@ -932,8 +887,6 @@ The `AppComponent`, for example, displays a navigation bar with anchors and thei -While the `AppComponent` *class* is empty, you might want to write unit tests to confirm that the links are wired properly to the `RouterLink` directives, perhaps for the reasons as explained in the [following section](#why-stubbed-routerlink-tests). - To validate the links, you don't need the `Router` to navigate and you don't need the `` to mark where the `Router` inserts *routed components*. The `BannerComponent` and `WelcomeComponent` \(indicated by `` and ``\) are also irrelevant. @@ -966,8 +919,6 @@ Then declare them in the `TestBed` configuration next to the components, directi The `AppComponent` is the test subject, so of course you declare the real version. -The `RouterLinkDirectiveStub`, [described later](#routerlink), is a test version of the real `RouterLink` that helps with the link tests. - The rest are stubs. @@ -980,7 +931,7 @@ In the second approach, add `NO_ERRORS_SCHEMA` to the `TestBed.schemas` metadata The `NO_ERRORS_SCHEMA` tells the Angular compiler to ignore unrecognized elements and attributes. -The compiler recognizes the `` element and the `routerLink` attribute because you declared a corresponding `AppComponent` and `RouterLinkDirectiveStub` in the `TestBed` configuration. +The compiler recognizes the `` element and the `routerLink` attribute because you declared a corresponding `AppComponent` and `RouterLink` in the `TestBed` configuration. But the compiler won't throw an error when it encounters ``, ``, or ``. It simply renders them as empty tags and the browser ignores them. @@ -1003,31 +954,7 @@ In practice you will combine the two techniques in the same setup, as seen in th -The Angular compiler creates the `BannerStubComponent` for the `` element and applies the `RouterLinkStubDirective` to the anchors with the `routerLink` attribute, but it ignores the `` and `` tags. - - - -## Components with `RouterLink` - -The real `RouterLinkDirective` is quite complicated and entangled with other components and directives of the `RouterModule`. -It requires challenging setup to mock and use in tests. - -The `RouterLinkDirectiveStub` in this sample code replaces the real directive with an alternative version designed to validate the kind of anchor tag wiring seen in the `AppComponent` template. - - - -The URL bound to the `[routerLink]` attribute flows in to the directive's `linkParams` property. - -The `HostListener` wires the click event of the host element \(the `` anchor elements in `AppComponent`\) to the stub directive's `onClick` method. - -Clicking the anchor should trigger the `onClick()` method, which sets the stub's telltale `navigatedTo` property. -Tests inspect `navigatedTo` to confirm that clicking the anchor sets the expected route definition. - -
- -Whether the router is configured properly to navigate with that route definition is a question for a separate set of tests. - -
+The Angular compiler creates the `BannerStubComponent` for the `` element and applies the `RouterLink` to the anchors with the `routerLink` attribute, but it ignores the `` and `` tags. @@ -1054,39 +981,6 @@ Here are some tests that confirm those links are wired to the `routerLink` direc -
- -The "click" test *in this example* is misleading. -It tests the `RouterLinkDirectiveStub` rather than the *component*. -This is a common failing of directive stubs. - -It has a legitimate purpose in this guide. -It demonstrates how to find a `RouterLink` element, click it, and inspect a result, without engaging the full router machinery. -This is a skill you might need to test a more sophisticated component, one that changes the display, re-calculates parameters, or re-arranges navigation options when the user clicks the link. - -
- - - -#### What good are these tests? - -Stubbed `RouterLink` tests can confirm that a component with links and an outlet is set up properly, that the component has the links it should have, and that they are all pointing in the expected direction. -These tests do not concern whether the application will succeed in navigating to the target component when the user clicks a link. - -Stubbing the RouterLink and RouterOutlet is the best option for such limited testing goals. -Relying on the real router would make them brittle. -They could fail for reasons unrelated to the component. -For example, a navigation guard could prevent an unauthorized user from visiting the `HeroListComponent`. -That's not the fault of the `AppComponent` and no change to that component could cure the failed test. - -A *different* battery of tests can explore whether the application navigates as expected in the presence of conditions that influence guards such as whether the user is authenticated and authorized. - -
- -A future guide update explains how to write such tests with the `RouterTestingModule`. - -
- ## Use a `page` object @@ -1126,9 +1020,6 @@ A `createComponent` method creates a `page` object and fills in the blanks once -The [`HeroDetailComponent` tests](#tests-w-test-double) in an earlier section demonstrate how `createComponent` and `page` keep the tests short and *on message*. -There are no distractions: no waiting for promises to resolve and no searching the DOM for element values to compare. - Here are a few more `HeroDetailComponent` tests to reinforce the point. @@ -1263,8 +1154,8 @@ In addition to the support it receives from the default testing module `CommonMo * `NgModel` and friends in the `FormsModule` to enable two-way data binding * The `TitleCasePipe` from the `shared` folder -* The Router services that these tests are stubbing out -* The Hero data access services that are also stubbed out +* The Router services +* The Hero data access services One approach is to configure the testing module from the individual pieces as in this example: @@ -1297,7 +1188,6 @@ Try a test configuration that imports the `HeroModule` like this one: -That's *really* crisp. Only the *test doubles* in the `providers` remain. Even the `HeroDetailComponent` declaration is gone. From de48c99afb77594b53d413445b997f6eb368a84f Mon Sep 17 00:00:00 2001 From: Kristiyan Kostadinov Date: Thu, 2 Mar 2023 14:19:27 +0100 Subject: [PATCH 09/32] refactor(migrations): expose current file in import remapper (#49288) Passes the path of the current file to the import remapper. Useful if we want to generate absolute paths. PR Close #49288 --- .../core/schematics/ng-generate/standalone-migration/util.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/core/schematics/ng-generate/standalone-migration/util.ts b/packages/core/schematics/ng-generate/standalone-migration/util.ts index 94be0d3ab626..1fa0fa16802a 100644 --- a/packages/core/schematics/ng-generate/standalone-migration/util.ts +++ b/packages/core/schematics/ng-generate/standalone-migration/util.ts @@ -24,7 +24,7 @@ export type NodeLookup = Map; export type NamedClassDeclaration = ts.ClassDeclaration&{name: ts.Identifier}; /** Function that can be used to remap a generated import. */ -export type ImportRemapper = (moduleName: string) => string; +export type ImportRemapper = (moduleName: string, inFile: string) => string; /** Text span of an AST node. */ export type ReferenceSpan = [start: number, end: number]; @@ -116,7 +116,7 @@ export class ChangeTracker { sourceFile: ts.SourceFile, symbolName: string, moduleName: string, alias: string|null = null): ts.Expression { if (this._importRemapper) { - moduleName = this._importRemapper(moduleName); + moduleName = this._importRemapper(moduleName, sourceFile.fileName); } // It's common for paths to be manipulated with Node's `path` utilties which From 13e675ea8753b505d8801d9332a1cb6330b4a800 Mon Sep 17 00:00:00 2001 From: Angular Robot Date: Tue, 28 Feb 2023 03:09:54 +0000 Subject: [PATCH 10/32] build: update all non-major dependencies (#49267) See associated pull request for more information. PR Close #49267 --- aio/package.json | 2 +- aio/yarn.lock | 31 ++++++++++++----- .../shell-browser/src/app/background.ts | 34 +++++++++++-------- package.json | 6 ++-- packages/bazel/package.json | 2 +- yarn.lock | 20 +++++------ 6 files changed, 56 insertions(+), 39 deletions(-) diff --git a/aio/package.json b/aio/package.json index d9b3ea0dab81..f455deae104c 100644 --- a/aio/package.json +++ b/aio/package.json @@ -145,7 +145,7 @@ "lunr": "^2.3.9", "npm-run-all": "^4.1.5", "protractor": "~7.0.0", - "puppeteer-core": "19.5.2", + "puppeteer-core": "19.7.2", "rehype-slug": "^4.0.1", "remark": "^12.0.0", "remark-html": "^13.0.0", diff --git a/aio/yarn.lock b/aio/yarn.lock index 59047a5b3ee6..f95d27059e8f 100644 --- a/aio/yarn.lock +++ b/aio/yarn.lock @@ -5117,6 +5117,13 @@ chrome-trace-event@^1.0.2: resolved "https://registry.yarnpkg.com/chrome-trace-event/-/chrome-trace-event-1.0.3.tgz#1015eced4741e15d06664a957dbbf50d041e26ac" integrity sha512-p3KULyQg4S7NIHixdwbGX+nFHkoBiA4YQmyWtjb8XngSKV124nJmRysgAeujbUVb15vh+RvFUfCPqU7rXk+hZg== +chromium-bidi@0.4.4: + version "0.4.4" + resolved "https://registry.yarnpkg.com/chromium-bidi/-/chromium-bidi-0.4.4.tgz#44f25d4fa5d2f3debc3fc3948d0657194cac4407" + integrity sha512-4BX5cSaponuvVT1+SbLYTOAgDoVtX/Khoc9UsbFJ/AsPVUeFAM3RiIDFI6XFhLYMi9WmVJqh1ZH+dRpNKkKwiQ== + dependencies: + mitt "3.0.0" + ci-info@^2.0.0: version "2.0.0" resolved "https://registry.yarnpkg.com/ci-info/-/ci-info-2.0.0.tgz#67a9e964be31a51e15e5010d58e6f12834002f46" @@ -5952,10 +5959,10 @@ dev-ip@^1.0.1: resolved "https://registry.yarnpkg.com/dev-ip/-/dev-ip-1.0.1.tgz#a76a3ed1855be7a012bb8ac16cb80f3c00dc28f0" integrity sha512-LmVkry/oDShEgSZPNgqCIp2/TlqtExeGmymru3uCELnfyjY11IzpAproLYs+1X88fXO6DBoYP3ul2Xo2yz2j6A== -devtools-protocol@0.0.1068969: - version "0.0.1068969" - resolved "https://registry.yarnpkg.com/devtools-protocol/-/devtools-protocol-0.0.1068969.tgz#8b9a4bc48aed1453bed08d62b07481f9abf4d6d8" - integrity sha512-ATFTrPbY1dKYhPPvpjtwWKSK2mIwGmRwX54UASn9THEuIZCe2n9k3vVuMmt6jWeL+e5QaaguEv/pMyR+JQB7VQ== +devtools-protocol@0.0.1094867: + version "0.0.1094867" + resolved "https://registry.yarnpkg.com/devtools-protocol/-/devtools-protocol-0.0.1094867.tgz#2ab93908e9376bd85d4e0604aa2651258f13e374" + integrity sha512-pmMDBKiRVjh0uKK6CT1WqZmM3hBVSgD+N2MrgyV1uNizAZMw4tx6i/RTc+/uCsKSCmg0xXx7arCP/OFcIwTsiQ== devtools-protocol@0.0.981744: version "0.0.981744" @@ -10308,6 +10315,11 @@ minizlib@^2.1.1, minizlib@^2.1.2: minipass "^3.0.0" yallist "^4.0.0" +mitt@3.0.0: + version "3.0.0" + resolved "https://registry.yarnpkg.com/mitt/-/mitt-3.0.0.tgz#69ef9bd5c80ff6f57473e8d89326d01c414be0bd" + integrity sha512-7dX2/10ITVyqh4aOSVI9gdape+t9l2/8QxHrFmUXu4EEUpdlxl6RudZUPZoc+zuY2hk1j7XxVroIVIan/pD/SQ== + mitt@^1.1.3: version "1.2.0" resolved "https://registry.yarnpkg.com/mitt/-/mitt-1.2.0.tgz#cb24e6569c806e31bd4e3995787fe38a04fdf90d" @@ -11527,14 +11539,15 @@ pupa@^2.0.1, pupa@^2.1.1: dependencies: escape-goat "^2.0.0" -puppeteer-core@19.5.2: - version "19.5.2" - resolved "https://registry.yarnpkg.com/puppeteer-core/-/puppeteer-core-19.5.2.tgz#9b454b0ef89d3f07e20158dd4ced6ebd85d4dadb" - integrity sha512-Rqk+3kqM+Z2deooTYqcYt8lRtGffJdifWa9td9nbJSjhANWsFouk8kLBNUKycewCCFHM8TZUKS0x28OllavW2A== +puppeteer-core@19.7.2: + version "19.7.2" + resolved "https://registry.yarnpkg.com/puppeteer-core/-/puppeteer-core-19.7.2.tgz#deee9ef915829b6a1d1a3a008625c29eeb251161" + integrity sha512-PvI+fXqgP0uGJxkyZcX51bnzjFA73MODZOAv0fSD35yR7tvbqwtMV3/Y+hxQ0AMMwzxkEebP6c7po/muqxJvmQ== dependencies: + chromium-bidi "0.4.4" cross-fetch "3.1.5" debug "4.3.4" - devtools-protocol "0.0.1068969" + devtools-protocol "0.0.1094867" extract-zip "2.0.1" https-proxy-agent "5.0.1" proxy-from-env "1.1.0" diff --git a/devtools/projects/shell-browser/src/app/background.ts b/devtools/projects/shell-browser/src/app/background.ts index d67ffc699c6e..9e7bd12a346d 100644 --- a/devtools/projects/shell-browser/src/app/background.ts +++ b/devtools/projects/shell-browser/src/app/background.ts @@ -26,13 +26,15 @@ const browserAction = (() => { // By default use the black and white icon. // Replace it only when we detect an Angular app. -browserAction.setIcon({ - path: { - 16: chrome.runtime.getURL(`assets/icon-bw16.png`), - 48: chrome.runtime.getURL(`assets/icon-bw48.png`), - 128: chrome.runtime.getURL(`assets/icon-bw128.png`), - }, -}); +browserAction.setIcon( + { + path: { + 16: chrome.runtime.getURL(`assets/icon-bw16.png`), + 48: chrome.runtime.getURL(`assets/icon-bw48.png`), + 128: chrome.runtime.getURL(`assets/icon-bw128.png`), + }, + }, + () => {}); const ports: { [tab: string]: @@ -181,13 +183,15 @@ chrome.runtime.onMessage.addListener((req, sender) => { }); } if (sender && sender.tab && req.isAngular) { - browserAction.setIcon({ - tabId: sender.tab.id, - path: { - 16: chrome.runtime.getURL(`assets/icon16.png`), - 48: chrome.runtime.getURL(`assets/icon48.png`), - 128: chrome.runtime.getURL(`assets/icon128.png`), - }, - }); + browserAction.setIcon( + { + tabId: sender.tab.id, + path: { + 16: chrome.runtime.getURL(`assets/icon16.png`), + 48: chrome.runtime.getURL(`assets/icon48.png`), + 128: chrome.runtime.getURL(`assets/icon128.png`), + }, + }, + () => {}); } }); diff --git a/package.json b/package.json index 6135ece2f6bc..98da20c3687f 100644 --- a/package.json +++ b/package.json @@ -88,7 +88,7 @@ "@types/babel__traverse": "7.18.3", "@types/base64-js": "1.3.0", "@types/bluebird": "^3.5.27", - "@types/chrome": "^0.0.208", + "@types/chrome": "^0.0.218", "@types/convert-source-map": "^1.5.1", "@types/diff": "^5.0.0", "@types/filesystem": "^0.0.32", @@ -139,8 +139,8 @@ "karma-firefox-launcher": "^2.1.0", "karma-jasmine": "^5.0.0", "karma-requirejs": "^1.1.0", - "karma-sourcemap-loader": "^0.3.7", - "magic-string": "0.27.0", + "karma-sourcemap-loader": "^0.4.0", + "magic-string": "0.30.0", "memo-decorator": "^2.0.1", "ngx-flamegraph": "0.0.11", "nodejs-websocket": "^1.7.2", diff --git a/packages/bazel/package.json b/packages/bazel/package.json index 753939d7060a..580f0ad06f65 100644 --- a/packages/bazel/package.json +++ b/packages/bazel/package.json @@ -23,7 +23,7 @@ }, "dependencies": { "@microsoft/api-extractor": "^7.24.2", - "magic-string": "^0.27.0", + "magic-string": "^0.30.0", "tsickle": "^0.46.3", "tslib": "^2.3.0" }, diff --git a/yarn.lock b/yarn.lock index afe72e64ed81..ea6f0f83d082 100644 --- a/yarn.lock +++ b/yarn.lock @@ -4095,10 +4095,10 @@ "@types/node" "*" "@types/responselike" "*" -"@types/chrome@^0.0.208": - version "0.0.208" - resolved "https://registry.yarnpkg.com/@types/chrome/-/chrome-0.0.208.tgz#c52992e46723c783d3fd84a8b90dd8b3e87af67f" - integrity sha512-VDU/JnXkF5qaI7WBz14Azpa2VseZTgML0ia/g/B1sr9OfdOnHiH/zZ7P7qCDqxSlkqJh76/bPc8jLFcx8rHJmw== +"@types/chrome@^0.0.218": + version "0.0.218" + resolved "https://registry.yarnpkg.com/@types/chrome/-/chrome-0.0.218.tgz#d354cf0ac85204c17cfbe03ba7eb8f58c75d03a4" + integrity sha512-GC/c9B3Eo3h3l+fV5G6A9cRMEZOYo46E21mjHHfkXz+/A5uypXstMVAMk04IeGY2DJ7PxVxbn26oFBPZOa4Dkw== dependencies: "@types/filesystem" "*" "@types/har-format" "*" @@ -9885,7 +9885,7 @@ got@^9.6.0: to-readable-stream "^1.0.0" url-parse-lax "^3.0.0" -graceful-fs@4.2.10, graceful-fs@^4.0.0, graceful-fs@^4.1.10, graceful-fs@^4.1.11, graceful-fs@^4.1.2, graceful-fs@^4.1.6, graceful-fs@^4.2.0, graceful-fs@^4.2.2, graceful-fs@^4.2.4, graceful-fs@^4.2.6, graceful-fs@^4.2.9: +graceful-fs@4.2.10, graceful-fs@^4.0.0, graceful-fs@^4.1.10, graceful-fs@^4.1.11, graceful-fs@^4.1.2, graceful-fs@^4.1.6, graceful-fs@^4.2.0, graceful-fs@^4.2.10, graceful-fs@^4.2.2, graceful-fs@^4.2.4, graceful-fs@^4.2.6, graceful-fs@^4.2.9: version "4.2.10" resolved "https://registry.yarnpkg.com/graceful-fs/-/graceful-fs-4.2.10.tgz#147d3a006da4ca3ce14728c7aefc287c367d7a6c" integrity sha512-9ByhssR2fPVsNZj478qUUbKfmL0+t5BDVyjShtyZZLiK7ZDAArFFfopyOTj0M05wE2tJPisA4iTnnXl2YoPvOA== @@ -11386,12 +11386,12 @@ karma-source-map-support@1.4.0: dependencies: source-map-support "^0.5.5" -karma-sourcemap-loader@^0.3.7: - version "0.3.8" - resolved "https://registry.yarnpkg.com/karma-sourcemap-loader/-/karma-sourcemap-loader-0.3.8.tgz#d4bae72fb7a8397328a62b75013d2df937bdcf9c" - integrity sha512-zorxyAakYZuBcHRJE+vbrK2o2JXLFWK8VVjiT/6P+ltLBUGUvqTEkUiQ119MGdOrK7mrmxXHZF1/pfT6GgIZ6g== +karma-sourcemap-loader@^0.4.0: + version "0.4.0" + resolved "https://registry.yarnpkg.com/karma-sourcemap-loader/-/karma-sourcemap-loader-0.4.0.tgz#b01d73f8f688f533bcc8f5d273d43458e13b5488" + integrity sha512-xCRL3/pmhAYF3I6qOrcn0uhbQevitc2DERMPH82FMnG+4WReoGcGFZb1pURf2a5apyrOHRdvD+O6K7NljqKHyA== dependencies: - graceful-fs "^4.1.2" + graceful-fs "^4.2.10" karma@~6.4.0: version "6.4.0" From f321e65c467ff0781af343035f8059945eefff3d Mon Sep 17 00:00:00 2001 From: Matthieu Riegler Date: Thu, 2 Mar 2023 22:12:59 +0100 Subject: [PATCH 11/32] refactor(common): remove `BrowserPlatformLocation` from private exports. (#49301) `BrowserPlatformLocation` was add to the public API by #48488. PR Close #49301 --- packages/common/src/private_export.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/common/src/private_export.ts b/packages/common/src/private_export.ts index ed72cf357aff..c4b334cd1773 100644 --- a/packages/common/src/private_export.ts +++ b/packages/common/src/private_export.ts @@ -7,4 +7,3 @@ */ export {DomAdapter as ɵDomAdapter, getDOM as ɵgetDOM, setRootDomAdapter as ɵsetRootDomAdapter} from './dom_adapter'; -export {BrowserPlatformLocation as ɵBrowserPlatformLocation} from './location/platform_location'; From b0a9e674199e0222b571cb92c1d784794dfec853 Mon Sep 17 00:00:00 2001 From: Matthieu Riegler Date: Fri, 3 Mar 2023 11:59:27 +0100 Subject: [PATCH 12/32] docs: fix links on untyped forms (#49306) PR Close #49306 --- packages/forms/src/form_builder.ts | 4 ++-- packages/forms/src/model/form_array.ts | 2 +- packages/forms/src/model/form_control.ts | 2 +- packages/forms/src/model/form_group.ts | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/forms/src/form_builder.ts b/packages/forms/src/form_builder.ts index e2e2ce769f67..f4eeb68cedb8 100644 --- a/packages/forms/src/form_builder.ts +++ b/packages/forms/src/form_builder.ts @@ -112,7 +112,7 @@ export class FormBuilder { /** * @description - * Returns a FormBuilder in which automatically constructed @see FormControl} elements + * Returns a FormBuilder in which automatically constructed `FormControl` elements * have `{nonNullable: true}` and are non-nullable. * * **Constructing non-nullable controls** @@ -413,7 +413,7 @@ export abstract class NonNullableFormBuilder { } /** - * UntypedFormBuilder is the same as @see FormBuilder, but it provides untyped controls. + * UntypedFormBuilder is the same as `FormBuilder`, but it provides untyped controls. */ @Injectable({providedIn: 'root'}) export class UntypedFormBuilder extends FormBuilder { diff --git a/packages/forms/src/model/form_array.ts b/packages/forms/src/model/form_array.ts index ad0eb86cf98b..6acd52e55f9e 100644 --- a/packages/forms/src/model/form_array.ts +++ b/packages/forms/src/model/form_array.ts @@ -523,7 +523,7 @@ interface UntypedFormArrayCtor { } /** - * UntypedFormArray is a non-strongly-typed version of @see FormArray, which + * UntypedFormArray is a non-strongly-typed version of `FormArray`, which * permits heterogenous controls. */ export type UntypedFormArray = FormArray; diff --git a/packages/forms/src/model/form_control.ts b/packages/forms/src/model/form_control.ts index 32baac2154ee..89663d30df76 100644 --- a/packages/forms/src/model/form_control.ts +++ b/packages/forms/src/model/form_control.ts @@ -555,7 +555,7 @@ interface UntypedFormControlCtor { } /** - * UntypedFormControl is a non-strongly-typed version of @see FormControl. + * UntypedFormControl is a non-strongly-typed version of `FormControl`. */ export type UntypedFormControl = FormControl; diff --git a/packages/forms/src/model/form_group.ts b/packages/forms/src/model/form_group.ts index aa612aa29260..6d59adee7ee2 100644 --- a/packages/forms/src/model/form_group.ts +++ b/packages/forms/src/model/form_group.ts @@ -598,7 +598,7 @@ interface UntypedFormGroupCtor { } /** - * UntypedFormGroup is a non-strongly-typed version of @see FormGroup. + * UntypedFormGroup is a non-strongly-typed version of `FormGroup`. */ export type UntypedFormGroup = FormGroup; From 6b9ec0b83065e98a40d2cf6a27ca5c66a7d7c16f Mon Sep 17 00:00:00 2001 From: Matthieu Riegler Date: Thu, 2 Mar 2023 19:38:26 +0100 Subject: [PATCH 13/32] =?UTF-8?q?refactor(core):=20Remove=20=C9=B5ivyEnabl?= =?UTF-8?q?ed=20(#49296)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The unit tests have been updated in #44120, this export is unused now. PR Close #49296 --- packages/core/src/core_private_export.ts | 3 --- 1 file changed, 3 deletions(-) diff --git a/packages/core/src/core_private_export.ts b/packages/core/src/core_private_export.ts index eb0026ac1c23..517aa08bf9f5 100644 --- a/packages/core/src/core_private_export.ts +++ b/packages/core/src/core_private_export.ts @@ -34,6 +34,3 @@ export {global as ɵglobal} from './util/global'; export {isObservable as ɵisObservable, isPromise as ɵisPromise, isSubscribable as ɵisSubscribable} from './util/lang'; export {stringify as ɵstringify} from './util/stringify'; export {NOT_FOUND_CHECK_ONLY_ELEMENT_INJECTOR as ɵNOT_FOUND_CHECK_ONLY_ELEMENT_INJECTOR} from './view/provider_flags'; - -// TODO(alxhub): allows tests to compile, can be removed when tests have been updated. -export const ɵivyEnabled = true; From 05db4797e8b14b84206a8a5a0fbd96ce9ce05310 Mon Sep 17 00:00:00 2001 From: Paul Gschwendtner Date: Thu, 2 Mar 2023 16:11:07 +0000 Subject: [PATCH 14/32] build: format `run-example-e2e.mjs` with prettier (#49293) Formats the `run-example-e2e.mjs` file with prettier to ease future diffs. PR Close #49293 --- aio/tools/examples/run-example-e2e.mjs | 60 +++++++++++++++----------- 1 file changed, 36 insertions(+), 24 deletions(-) diff --git a/aio/tools/examples/run-example-e2e.mjs b/aio/tools/examples/run-example-e2e.mjs index 217863299baf..32188b9ff5b9 100644 --- a/aio/tools/examples/run-example-e2e.mjs +++ b/aio/tools/examples/run-example-e2e.mjs @@ -1,15 +1,17 @@ import path from 'canonical-path'; import {spawn} from 'cross-spawn'; import fs from 'fs-extra'; +import getPort from 'get-port'; import {globbySync} from 'globby'; import os from 'os'; import shelljs from 'shelljs'; import treeKill from 'tree-kill'; import yargs from 'yargs'; import {hideBin} from 'yargs/helpers' -import getPort from 'get-port'; -import {constructExampleSandbox} from "./example-sandbox.mjs"; -import { getAdjustedChromeBinPathForWindows } from '../windows-chromium-path.js'; + +import {getAdjustedChromeBinPathForWindows} from '../windows-chromium-path.js'; + +import {constructExampleSandbox} from './example-sandbox.mjs'; shelljs.set('-e'); @@ -20,11 +22,17 @@ process.env.CHROME_BIN = getAdjustedChromeBinPathForWindows(); process.env.CHROME_BIN = path.resolve(process.env.CHROME_BIN); process.env.CHROMEDRIVER_BIN = path.resolve(process.env.CHROMEDRIVER_BIN); -const {argv} = yargs(hideBin(process.argv)) - .command("* ") - .option('localPackage', {array: true, type: 'string', default: [], describe: 'Locally built package to substitute, in the form `packageName#packagePath`'}) - .version(false) - .strict(); +const {argv} = + yargs(hideBin(process.argv)) + .command('* ') + .option('localPackage', { + array: true, + type: 'string', + default: [], + describe: 'Locally built package to substitute, in the form `packageName#packagePath`' + }) + .version(false) + .strict(); const EXAMPLE_PATH = path.resolve(argv.examplePath); const NODE = process.execPath; @@ -44,13 +52,15 @@ const MAX_NO_OUTPUT_TIMEOUT = 1000 * 60 * 5; // 5 minutes /** * Run Protractor End-to-End Tests for a Docs Example * - * Usage: node run-example-e2e.mjs [localPackage...] + * Usage: node run-example-e2e.mjs + * [localPackage...] * * Args: * examplePath: path to the example * yarnPath: path to a vendored version of yarn * exampleDepsWorkspaceName: name of bazel workspace containing example node_omodules - * localPackages: a vararg of local packages to substitute in place npm deps, in the form @package/name#pathToPackage. + * localPackages: a vararg of local packages to substitute in place npm deps, in the + * form @package/name#pathToPackage. * * Flags * --retry to retry failed tests (useful for overcoming flakes) @@ -63,8 +73,9 @@ async function runE2e(examplePath) { const exampleTestPath = generatePathForExampleTest(exampleName); try { - - await constructExampleSandbox(examplePath, exampleTestPath, path.resolve('..', EXAMPLE_DEPS_WORKSPACE_NAME, 'node_modules'), LOCAL_PACKAGES); + await constructExampleSandbox( + examplePath, exampleTestPath, + path.resolve('..', EXAMPLE_DEPS_WORKSPACE_NAME, 'node_modules'), LOCAL_PACKAGES); let testFn; if (isSystemJsTest(exampleTestPath)) { @@ -74,7 +85,7 @@ async function runE2e(examplePath) { } else { throw new Error(`Unknown e2e test type for example ${exampleName}`); } - + await attempt(testFn, maxAttempts); } catch (e) { console.error(e); @@ -147,7 +158,7 @@ async function overrideSystemJsExampleToUseRandomPort(exampleConfig, exampleDir) const isUsingHttpServerLibrary = exampleConfig.run === 'serve:upgrade'; if (isUsingHttpServerLibrary) { // Override the port in http-server by passing as an argument - runArgs = [...runArgs, "-p", freePort]; + runArgs = [...runArgs, '-p', freePort]; } // Override the port in any lite-server config files @@ -162,7 +173,8 @@ async function overrideSystemJsExampleToUseRandomPort(exampleConfig, exampleDir) // Override hardcoded port in protractor.config.js let protractorConfig = fs.readFileSync(path.join(exampleDir, 'protractor.config.js'), 'utf8'); - protractorConfig = protractorConfig.replaceAll('http://localhost:8080', `http://localhost:${freePort}`); + protractorConfig = + protractorConfig.replaceAll('http://localhost:8080', `http://localhost:${freePort}`); fs.writeFileSync(path.join(exampleDir, 'protractor.config.js'), protractorConfig); return runArgs; @@ -195,9 +207,8 @@ function runProtractorSystemJS(exampleName, prepPromise, appDir, appRunSpawnInfo }); }) .then( - () => finish(appRunSpawnInfo.proc.pid, true), - () => finish(appRunSpawnInfo.proc.pid, false) - ); + () => finish(appRunSpawnInfo.proc.pid, true), + () => finish(appRunSpawnInfo.proc.pid, false)); } function finish(spawnProcId, ok) { @@ -229,7 +240,8 @@ function runE2eTestsCLI(exampleName, appDir) { const config = loadExampleConfig(appDir); - // Replace any calls with yarn (which requires yarn to be on the PATH) to instead call our vendored yarn + // Replace any calls with yarn (which requires yarn to be on the PATH) to instead call our + // vendored yarn if (config.tests) { for (let test of config.tests) { if (test.cmd === 'yarn') { @@ -241,9 +253,9 @@ function runE2eTestsCLI(exampleName, appDir) { // `--no-webdriver-update` is needed to preserve the ChromeDriver version already installed. const testCommands = config.tests || [{ - cmd: NODE, + cmd: NODE, args: [ - VENDORED_YARN, + VENDORED_YARN, 'e2e', '--configuration=production', '--protractor-config=e2e/protractor-bazel.conf.js', @@ -254,8 +266,7 @@ function runE2eTestsCLI(exampleName, appDir) { const e2eSpawnPromise = testCommands.reduce((prevSpawnPromise, {cmd, args}) => { return prevSpawnPromise.then(() => { - const currSpawn = spawnExt( - cmd, args, {cwd: appDir}, false); + const currSpawn = spawnExt(cmd, args, {cwd: appDir}, false); return currSpawn.promise.then( () => finish(currSpawn.proc.pid, true), () => finish(currSpawn.proc.pid, false), @@ -268,7 +279,8 @@ function runE2eTestsCLI(exampleName, appDir) { // Returns both a promise and the spawned process so that it can be killed if needed. function spawnExt( - command, args, options, ignoreClose = false, printMessageFn = msg => process.stdout.write(msg)) { + command, args, options, ignoreClose = false, + printMessageFn = msg => process.stdout.write(msg)) { let proc = null; const promise = new Promise((resolveFn, rejectFn) => { let noOutputTimeoutId; From b23fa5e977cdb6ecf2e199795b3354f45081d3af Mon Sep 17 00:00:00 2001 From: Paul Gschwendtner Date: Thu, 2 Mar 2023 16:42:57 +0000 Subject: [PATCH 15/32] build(docs-infra): fix example e2e tests not failing properly in some situations (#49293) The `run-example-e2e` script does not properly fail if configured tests of examples are failing. This happens when a CLI example configures multiple tests in the `example-config`. Due to incorrect usage of promises in combination with reduce, only the last test command had an effect on the overall test conclusion. A similar issue seems to occur with SystemJS Protractor tests. This commit fixes the problem and also cleans up the code a little by switching it to `async/await`. PR Close #49293 --- aio/tools/examples/run-example-e2e.mjs | 83 ++++++++++---------------- 1 file changed, 30 insertions(+), 53 deletions(-) diff --git a/aio/tools/examples/run-example-e2e.mjs b/aio/tools/examples/run-example-e2e.mjs index 32188b9ff5b9..0547064dab2c 100644 --- a/aio/tools/examples/run-example-e2e.mjs +++ b/aio/tools/examples/run-example-e2e.mjs @@ -101,7 +101,12 @@ async function attempt(testFn, maxAttempts) { while (true) { attempts++; - passed = await testFn(); + passed = true; + try { + await testFn(); + } catch (e) { + passed = false; + } if (passed || (attempts >= maxAttempts)) break; } @@ -136,13 +141,15 @@ async function runE2eTestsSystemJS(exampleName, appDir) { const appBuildSpawnInfo = spawnExt(NODE, [VENDORED_YARN, config.build], {cwd: appDir}); const appRunSpawnInfo = spawnExt(NODE, [VENDORED_YARN, ...runArgs, '-s'], {cwd: appDir}, true); - let run = runProtractorSystemJS(exampleName, appBuildSpawnInfo.promise, appDir, appRunSpawnInfo); + try { + await runProtractorSystemJS(exampleName, appBuildSpawnInfo.promise, appDir); + } finally { + treeKill(appRunSpawnInfo.proc.pid); + } if (fs.existsSync(appDir + '/aot/index.html')) { - run = run.then((ok) => ok && runProtractorAoT(exampleName, appDir)); + await runProtractorAoT(exampleName, appDir); } - - return run; } // The SystemJS examples spawn an http server and protractor using a hardcoded @@ -180,46 +187,16 @@ async function overrideSystemJsExampleToUseRandomPort(exampleConfig, exampleDir) return runArgs; } -function runProtractorSystemJS(exampleName, prepPromise, appDir, appRunSpawnInfo) { - const specFilename = path.resolve(`${appDir}/${SJS_SPEC_FILENAME}`); - return prepPromise - .catch(() => { - const emsg = `Application at ${appDir} failed to transpile.\n\n`; - console.log(emsg); - return Promise.reject(emsg); - }) - .then(() => { - let transpileError = false; - - // Start protractor. - console.log(`\n\n=========== Running aio example tests for: ${exampleName}`); - const spawnInfo = spawnExt(NODE, [VENDORED_YARN, 'protractor'], {cwd: appDir}); - - spawnInfo.proc.stderr.on('data', function(data) { - transpileError = transpileError || /npm ERR! Exit status 100/.test(data.toString()); - }); - return spawnInfo.promise.catch(function() { - if (transpileError) { - const emsg = `${specFilename} failed to transpile.\n\n`; - console.log(emsg); - } - return Promise.reject(); - }); - }) - .then( - () => finish(appRunSpawnInfo.proc.pid, true), - () => finish(appRunSpawnInfo.proc.pid, false)); -} +async function runProtractorSystemJS(exampleName, prepPromise, appDir) { + await prepPromise; -function finish(spawnProcId, ok) { - // Ugh... proc.kill does not work properly on windows with child processes. - // appRun.proc.kill(); - treeKill(spawnProcId); - return ok; + // Wait for the app to be running. Then we can start Protractor tests. + console.log(`\n\n=========== Running aio example tests for: ${exampleName}`); + await spawnExt(NODE, [VENDORED_YARN, 'protractor'], {cwd: appDir}).promise; } // Run e2e tests over the AOT build for projects that examples it. -function runProtractorAoT(exampleName, appDir) { +async function runProtractorAoT(exampleName, appDir) { const aotBuildSpawnInfo = spawnExt(NODE, [VENDORED_YARN, 'build:aot'], {cwd: appDir}); let promise = aotBuildSpawnInfo.promise; @@ -227,15 +204,22 @@ function runProtractorAoT(exampleName, appDir) { if (fs.existsSync(appDir + '/' + copyFileCmd)) { promise = promise.then(() => spawnExt('node', [copyFileCmd], {cwd: appDir}).promise); } + + // Run the server in the background. Will be killed upon test completion. const aotRunSpawnInfo = spawnExt(NODE, [VENDORED_YARN, 'serve:aot'], {cwd: appDir}, true); - return runProtractorSystemJS(exampleName, promise, appDir, aotRunSpawnInfo); + + try { + await runProtractorSystemJS(exampleName, promise, appDir); + } finally { + treeKill(aotRunSpawnInfo.proc.pid); + } } // Start the example in appDir; then run protractor with the specified // fileName; then shut down the example. // All protractor output is appended to the outputFile. // CLI version -function runE2eTestsCLI(exampleName, appDir) { +async function runE2eTestsCLI(exampleName, appDir) { console.log(`\n\n=========== Running aio example tests for: ${exampleName}`); const config = loadExampleConfig(appDir); @@ -264,17 +248,10 @@ function runE2eTestsCLI(exampleName, appDir) { ], }]; - const e2eSpawnPromise = testCommands.reduce((prevSpawnPromise, {cmd, args}) => { - return prevSpawnPromise.then(() => { - const currSpawn = spawnExt(cmd, args, {cwd: appDir}, false); - return currSpawn.promise.then( - () => finish(currSpawn.proc.pid, true), - () => finish(currSpawn.proc.pid, false), - ) - }); - }, Promise.resolve()); - return e2eSpawnPromise; + for (const {cmd, args} of testCommands) { + await spawnExt(cmd, args, {cwd: appDir}, false).promise; + } } // Returns both a promise and the spawned process so that it can be killed if needed. From 5e85c138fae246c4fe15e14ddbd6510f57c78ab2 Mon Sep 17 00:00:00 2001 From: Paul Gschwendtner Date: Fri, 3 Mar 2023 13:33:42 +0000 Subject: [PATCH 16/32] test: fix node and dom types conflict in `practical-observable-usage` example (#49293) This causes the e2e tests to fail. The tests were compiled using a tsconfig with `lib=dom`, but the tests explicitly tried to pull in the Node types. This causes a conflict for e.g. `AbortController` types. PR Close #49293 --- .../practical-observable-usage/src/typeahead.spec.ts | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/aio/content/examples/practical-observable-usage/src/typeahead.spec.ts b/aio/content/examples/practical-observable-usage/src/typeahead.spec.ts index fc3b637e5d12..0f9b0aea3028 100644 --- a/aio/content/examples/practical-observable-usage/src/typeahead.spec.ts +++ b/aio/content/examples/practical-observable-usage/src/typeahead.spec.ts @@ -1,5 +1,3 @@ -/// - import { of } from 'rxjs'; import { docRegionTypeahead } from './typeahead'; @@ -71,7 +69,7 @@ describe('typeahead', () => { // Helpers interface MockTask { - id: NodeJS.Timeout; + id: ReturnType; fn: () => unknown; delay: number; recurring: boolean; @@ -86,12 +84,12 @@ describe('typeahead', () => { static install(mockTime = 0): MockClock['tick'] { const mocked = new this(mockTime); - spyOn(global, 'clearInterval').and.callFake(id => mocked.clearTask(id as MockTask['id'])); - spyOn(global, 'clearTimeout').and.callFake(id => mocked.clearTask(id as MockTask['id'])); - spyOn(global, 'setInterval').and.callFake( + spyOn(globalThis, 'clearInterval').and.callFake(id => mocked.clearTask(id as MockTask['id'])); + spyOn(globalThis, 'clearTimeout').and.callFake(id => mocked.clearTask(id as MockTask['id'])); + spyOn(globalThis, 'setInterval').and.callFake( ((fn: () => unknown, delay: number, ...args: any[]) => mocked.createTask(fn, delay, true, ...args)) as typeof setInterval); - spyOn(global, 'setTimeout').and.callFake( + spyOn(globalThis, 'setTimeout').and.callFake( ((fn: () => unknown, delay: number, ...args: any[]) => mocked.createTask(fn, delay, false, ...args)) as typeof setTimeout); From 5bb4407c01dac2696f1a757c0f5b2c7a2b6bd0ea Mon Sep 17 00:00:00 2001 From: Paul Gschwendtner Date: Fri, 3 Mar 2023 13:55:27 +0000 Subject: [PATCH 17/32] build: fix AIO local e2e examples picking up incorrect dependencies (#49293) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Whenever we run example tests using the local framework packages, the e2e tests will have the local framework packages symlinked in the `node_modules`. This works well in general, but due to NodeJS by default resolving symlinks to the target location, NodeJS will end up looking for transitive dependencies in the `bazel-bin` instead of in the example `node_modules` folder. This means that we end up incorrectly resolving older versions of `@angular/core` that end up existing in the main project dependencies. This causes errors like: ``` Error: ../../home/circleci/.cache/bazel/_bazel_circleci/9ce5c2144ecf75d11717c0aa41e45a8d/execroot/angular/bazel-out/k8-fastbuild/bin/packages/common/npm_package/http/testing/index.d.ts:12:21 - error TS2307: Cannot find module '@angular/common/http' or its corresponding type declarations. 12 import * as i1 from '@angular/common/http'; ~~~~~~~~~~~~~~~~~~~~~~ Error: ../../home/circleci/.cache/bazel/_bazel_circleci/9ce5c2144ecf75d11717c0aa41e45a8d/execroot/angular/bazel-out/k8-fastbuild/bin/packages/common/npm_package/index.d.ts:1630:18 - error TS2707: Generic type 'ɵɵDirectiveDeclaration' requires between 6 and 8 type arguments. 1630 static ɵdir: i0.ɵɵDirectiveDeclaration; ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ``` We can fix this by properly ensuring that NodeJS does not resolve symlinks, but rather preserves them. In the error above, the e2e tests end up accidentally resolving `@angular/core` v14 that comes from `@angular/benchpress`. Angular Benchpress is installed via `@angular/build-tooling` in the project root. PR Close #49293 --- aio/tools/examples/example-sandbox.mjs | 25 +++++++++++-------------- 1 file changed, 11 insertions(+), 14 deletions(-) diff --git a/aio/tools/examples/example-sandbox.mjs b/aio/tools/examples/example-sandbox.mjs index 99227e04152f..dc83014ea4de 100644 --- a/aio/tools/examples/example-sandbox.mjs +++ b/aio/tools/examples/example-sandbox.mjs @@ -132,11 +132,13 @@ function pointBinSymlinksToLocalPackages(linkedNodeModules, exampleDepsNodeModul }); } -// When local packages are symlinked in, node has trouble resolving some peer deps. Setting -// preserveSymlinks in relevant files fixes this. This isn't required without local packages -// because in the worst case we would leak into the original Bazel repository and it would -// still find a node_modules folder for resolution. Add the preserveSymlinks options to various -// files that are used by the cli and systemjs tests (and sometimes both). +/** + * When local packages are symlinked in, node will by default resolve local packages to + * their output location in the `bazel-bin`. This will then cause transitive dependencies + * to be incorrectly resolved from `bazel-bin`, instead of from within the example sandbox. + * + * Setting `preserveSymlinks` in relevant files fixes this. + */ function preserveSymlinksWhenUsingLocalPackages(LOCAL_PACKAGES, appDir) { if (Object.keys(LOCAL_PACKAGES).length === 0) { return; @@ -145,19 +147,16 @@ function preserveSymlinksWhenUsingLocalPackages(LOCAL_PACKAGES, appDir) { // Set preserveSymlinks in angular.json const angularJsonPath = path.join(appDir, 'angular.json'); if (fs.existsSync(angularJsonPath)) { - const angularJson = jsonc.load(angularJsonPath, { - encoding: 'utf-8' - }); + const angularJson = jsonc.load(angularJsonPath, {encoding: 'utf-8'}); angularJson.projects['angular.io-example'].architect.build.options.preserveSymlinks = true; + angularJson.projects['angular.io-example'].architect.test.options.preserveSymlinks = true; fs.writeFileSync(angularJsonPath, JSON.stringify(angularJson, undefined, 2)); } // Set preserveSymlinks in any tsconfig.json files const tsConfigPaths = globbySync([path.join(appDir, 'tsconfig*.json')]); for (const tsConfigPath of tsConfigPaths) { - const tsConfig = jsonc.load(tsConfigPath, { - encoding: 'utf-8' - }); + const tsConfig = jsonc.load(tsConfigPath, {encoding: 'utf-8'}); const isRootConfig = !tsConfig.extends; if (isRootConfig) { tsConfig.compilerOptions.preserveSymlinks = true; @@ -167,9 +166,7 @@ function preserveSymlinksWhenUsingLocalPackages(LOCAL_PACKAGES, appDir) { // Call rollup with --preserveSymlinks const packageJsonPath = path.join(appDir, 'package.json'); - const packageJson = jsonc.load(packageJsonPath, { - encoding: 'utf-8' - }); + const packageJson = jsonc.load(packageJsonPath, {encoding: 'utf-8'}); if ('rollup' in packageJson.dependencies || 'rollup' in packageJson.devDependencies) { packageJson.scripts.rollup = 'rollup --preserveSymlinks'; fs.writeFileSync(packageJsonPath, JSON.stringify(packageJson, undefined, 2)); From f859de3e60487f6ac1b697800406b48026d38372 Mon Sep 17 00:00:00 2001 From: Paul Gschwendtner Date: Fri, 3 Mar 2023 14:18:08 +0000 Subject: [PATCH 18/32] build: ensure chromium is not using sandbox within Bazel sandbox for AIO example e2e tests (#49293) Chromium is launched via Karma from within the Bazel AIO example e2e tests. This breaks depending on the platform and sandbox mechanism used. We should never use Chromium's sandbox on top of Bazel's sandbox invocation. The Angular CLI exposes a browser exactly for this use-case. PR Close #49293 --- aio/content/examples/forms-overview/example-config.json | 2 +- aio/content/examples/http/example-config.json | 2 +- aio/content/examples/setup/example-config.json | 2 +- aio/content/examples/testing/example-config.json | 2 +- aio/content/examples/toh-pt6/example-config.json | 2 +- .../shared/boilerplate/cli/e2e/protractor-bazel.conf.js | 8 ++++++-- 6 files changed, 11 insertions(+), 7 deletions(-) diff --git a/aio/content/examples/forms-overview/example-config.json b/aio/content/examples/forms-overview/example-config.json index e2cf1e1e4e75..ccf472031b4d 100644 --- a/aio/content/examples/forms-overview/example-config.json +++ b/aio/content/examples/forms-overview/example-config.json @@ -1,6 +1,6 @@ { "tests": [ - {"cmd": "yarn", "args": ["test", "--browsers=ChromeHeadless", "--no-watch"]}, + {"cmd": "yarn", "args": ["test", "--browsers=ChromeHeadlessNoSandbox", "--no-watch"]}, {"cmd": "yarn", "args": ["e2e", "--configuration=production", "--protractor-config=e2e/protractor-bazel.conf.js", "--no-webdriver-update", "--port=0"]} ] } diff --git a/aio/content/examples/http/example-config.json b/aio/content/examples/http/example-config.json index a92715ec1cdb..239710e2cd0f 100644 --- a/aio/content/examples/http/example-config.json +++ b/aio/content/examples/http/example-config.json @@ -1,7 +1,7 @@ { "projectType": "testing", "tests": [ - {"cmd": "yarn", "args": ["test", "--browsers=ChromeHeadless", "--no-watch"]}, + {"cmd": "yarn", "args": ["test", "--browsers=ChromeHeadlessNoSandbox", "--no-watch"]}, {"cmd": "yarn", "args": ["e2e", "--configuration=production", "--protractor-config=e2e/protractor-bazel.conf.js", "--no-webdriver-update", "--port=0"]} ] } diff --git a/aio/content/examples/setup/example-config.json b/aio/content/examples/setup/example-config.json index e2cf1e1e4e75..ccf472031b4d 100644 --- a/aio/content/examples/setup/example-config.json +++ b/aio/content/examples/setup/example-config.json @@ -1,6 +1,6 @@ { "tests": [ - {"cmd": "yarn", "args": ["test", "--browsers=ChromeHeadless", "--no-watch"]}, + {"cmd": "yarn", "args": ["test", "--browsers=ChromeHeadlessNoSandbox", "--no-watch"]}, {"cmd": "yarn", "args": ["e2e", "--configuration=production", "--protractor-config=e2e/protractor-bazel.conf.js", "--no-webdriver-update", "--port=0"]} ] } diff --git a/aio/content/examples/testing/example-config.json b/aio/content/examples/testing/example-config.json index a92715ec1cdb..239710e2cd0f 100644 --- a/aio/content/examples/testing/example-config.json +++ b/aio/content/examples/testing/example-config.json @@ -1,7 +1,7 @@ { "projectType": "testing", "tests": [ - {"cmd": "yarn", "args": ["test", "--browsers=ChromeHeadless", "--no-watch"]}, + {"cmd": "yarn", "args": ["test", "--browsers=ChromeHeadlessNoSandbox", "--no-watch"]}, {"cmd": "yarn", "args": ["e2e", "--configuration=production", "--protractor-config=e2e/protractor-bazel.conf.js", "--no-webdriver-update", "--port=0"]} ] } diff --git a/aio/content/examples/toh-pt6/example-config.json b/aio/content/examples/toh-pt6/example-config.json index e2cf1e1e4e75..ccf472031b4d 100644 --- a/aio/content/examples/toh-pt6/example-config.json +++ b/aio/content/examples/toh-pt6/example-config.json @@ -1,6 +1,6 @@ { "tests": [ - {"cmd": "yarn", "args": ["test", "--browsers=ChromeHeadless", "--no-watch"]}, + {"cmd": "yarn", "args": ["test", "--browsers=ChromeHeadlessNoSandbox", "--no-watch"]}, {"cmd": "yarn", "args": ["e2e", "--configuration=production", "--protractor-config=e2e/protractor-bazel.conf.js", "--no-webdriver-update", "--port=0"]} ] } diff --git a/aio/tools/examples/shared/boilerplate/cli/e2e/protractor-bazel.conf.js b/aio/tools/examples/shared/boilerplate/cli/e2e/protractor-bazel.conf.js index 05e3032ce183..816c83bd84d4 100644 --- a/aio/tools/examples/shared/boilerplate/cli/e2e/protractor-bazel.conf.js +++ b/aio/tools/examples/shared/boilerplate/cli/e2e/protractor-bazel.conf.js @@ -13,8 +13,12 @@ exports.config = { chromeOptions: { ...config.capabilities.chromeOptions, binary: process.env.CHROME_BIN, - // See /integration/README.md#browser-tests for more info on these args - args: ['--no-sandbox', '--headless', '--disable-gpu', '--disable-dev-shm-usage', '--hide-scrollbars', '--mute-audio'], + // See /integration/README.md#browser-tests for more info on these args. + // Bazel tests run within a sandbox already and Chrome cannot have its own sandbox too. + args: [ + '--no-sandbox', '--headless', '--disable-gpu', '--disable-dev-shm-usage', + '--hide-scrollbars', '--mute-audio' + ], }, }, }; From 4ae753d84fe2fb89060b3bb212fd9acd39e9bf9a Mon Sep 17 00:00:00 2001 From: Paul Gschwendtner Date: Fri, 3 Mar 2023 15:05:02 +0000 Subject: [PATCH 19/32] build: always preserve symlinks when executing e2e test commands (#49293) Currently, examples with test commands like `ng build` are *never* using the local version of `//packages/compiler-cli`. This is because the CLI is invoked accidentally from within `external/aio_example_deps`. Since the CLI relies on importing the compiler-cli, it will always resolve the dependency from that directory- causing it to be always the version installed via `aio/examples/tools/shared/package.json`. We should never resolve symlinks and escape the e2e sandbox. That way the compiler-cli would be resolved properly and could also become the locally built one, depending on the test mode (i.e. npm or "local"). PR Close #49293 --- aio/tools/examples/example-sandbox.mjs | 12 +++++------- aio/tools/examples/run-example-e2e.mjs | 12 +++++++++++- 2 files changed, 16 insertions(+), 8 deletions(-) diff --git a/aio/tools/examples/example-sandbox.mjs b/aio/tools/examples/example-sandbox.mjs index dc83014ea4de..cef6b3ce476a 100644 --- a/aio/tools/examples/example-sandbox.mjs +++ b/aio/tools/examples/example-sandbox.mjs @@ -25,7 +25,7 @@ export async function constructExampleSandbox(examplePath, destPath, nodeModules await constructSymlinkedNodeModules(destPath, nodeModulesPath, localPackages); // Add preserveSymlinks fixups to various files --- needed when linkin in local deps - preserveSymlinksWhenUsingLocalPackages(localPackages, destPath); + ensurePreserveSymlinks(destPath); } async function constructSymlinkedNodeModules(examplePath, exampleDepsNodeModules, localPackages) { @@ -137,13 +137,11 @@ function pointBinSymlinksToLocalPackages(linkedNodeModules, exampleDepsNodeModul * their output location in the `bazel-bin`. This will then cause transitive dependencies * to be incorrectly resolved from `bazel-bin`, instead of from within the example sandbox. * - * Setting `preserveSymlinks` in relevant files fixes this. + * Setting `preserveSymlinks` in relevant files fixes this. Note that we are intending to + * preserve symlinks in general (regardless of local packages being used), because it + * allows us to safely enable `NODE_PRESERVE_SYMLINKS=1` when executing commands inside. */ -function preserveSymlinksWhenUsingLocalPackages(LOCAL_PACKAGES, appDir) { - if (Object.keys(LOCAL_PACKAGES).length === 0) { - return; - } - +function ensurePreserveSymlinks(appDir) { // Set preserveSymlinks in angular.json const angularJsonPath = path.join(appDir, 'angular.json'); if (fs.existsSync(angularJsonPath)) { diff --git a/aio/tools/examples/run-example-e2e.mjs b/aio/tools/examples/run-example-e2e.mjs index 0547064dab2c..924c49ebdec2 100644 --- a/aio/tools/examples/run-example-e2e.mjs +++ b/aio/tools/examples/run-example-e2e.mjs @@ -72,6 +72,8 @@ async function runE2e(examplePath) { const maxAttempts = argv.retry || 1; const exampleTestPath = generatePathForExampleTest(exampleName); + console.info('Running example tests in directory: ', exampleTestPath) + try { await constructExampleSandbox( examplePath, exampleTestPath, @@ -282,7 +284,15 @@ function spawnExt( let descr = command + ' ' + args.join(' '); printMessage(`running: ${descr}\n`); try { - proc = spawn(command, args, options); + proc = spawn(command, args, { + // All NodeJS scripts executed for running example e2e tests should preserve symlinks. + // This is important as otherwise test commands like `yarn ng build` would escape from the + // example sandbox into the `bazel-bin` where ultimately incorrect versions of local + // framework packages might be resolved. e.g. the `@angular/compiler-cli` version is never + // the one locally built. + env: {...process.env, NODE_PRESERVE_SYMLINKS: '1'}, + ...options + }); } catch (e) { console.log(e); return reject(e); From b90b3299aa7add8a68a9c1a71ef3be82b92eb326 Mon Sep 17 00:00:00 2001 From: Paul Gschwendtner Date: Fri, 3 Mar 2023 16:08:09 +0000 Subject: [PATCH 20/32] test: fix router testing example breaking due to non-existent stub (#49293) We recently migrated the testing example to use the router testing harness. There was one instance of the previous non-`TestBed` examples left that still relies on a stub that has already been removed. This example is not used anywhere and we should rather encourage a single pattern of testing. i.e. the harness as per recent changes. This commit removes the broken file. PR Close #49293 --- .../hero-detail.component.no-testbed.spec.ts | 66 ------------------- 1 file changed, 66 deletions(-) delete mode 100644 aio/content/examples/testing/src/app/hero/hero-detail.component.no-testbed.spec.ts diff --git a/aio/content/examples/testing/src/app/hero/hero-detail.component.no-testbed.spec.ts b/aio/content/examples/testing/src/app/hero/hero-detail.component.no-testbed.spec.ts deleted file mode 100644 index c0c55f56ad2a..000000000000 --- a/aio/content/examples/testing/src/app/hero/hero-detail.component.no-testbed.spec.ts +++ /dev/null @@ -1,66 +0,0 @@ -import { Router } from '@angular/router'; - -import { asyncData, ActivatedRouteStub } from '../../testing'; - -import { HeroDetailComponent } from './hero-detail.component'; -import { HeroDetailService } from './hero-detail.service'; -import { Hero } from '../model/hero'; - -////////// Tests //////////////////// - -describe('HeroDetailComponent - no TestBed', () => { - let comp: HeroDetailComponent; - let expectedHero: Hero; - let hds: jasmine.SpyObj; - let router: jasmine.SpyObj; - - beforeEach((done: DoneFn) => { - expectedHero = { id: 42, name: 'Bubba' }; - const activatedRoute = new ActivatedRouteStub({ id: expectedHero.id }); - router = jasmine.createSpyObj('Router', ['navigate']); - - hds = jasmine.createSpyObj('HeroDetailService', ['getHero', 'saveHero']); - hds.getHero.and.returnValue(asyncData(expectedHero)); - hds.saveHero.and.returnValue(asyncData(expectedHero)); - - comp = new HeroDetailComponent(hds, activatedRoute as any, router); - comp.ngOnInit(); - - // OnInit calls HDS.getHero; wait for it to get the fake hero - hds.getHero.calls.first().returnValue.subscribe(() => done()); - }); - - it('should expose the hero retrieved from the service', () => { - expect(comp.hero).toBe(expectedHero); - }); - - it('should navigate when click cancel', () => { - comp.cancel(); - expect(router.navigate.calls.any()) - .withContext('router.navigate called') - .toBe(true); - }); - - it('should save when click save', () => { - comp.save(); - expect(hds.saveHero.calls.any()) - .withContext('HeroDetailService.save called') - .toBe(true); - expect(router.navigate.calls.any()) - .withContext('router.navigate not called yet') - .toBe(false); - }); - - it('should navigate when click save resolves', (done: DoneFn) => { - comp.save(); - // waits for async save to complete before navigating - hds.saveHero.calls.first().returnValue - .subscribe(() => { - expect(router.navigate.calls.any()) - .withContext('router.navigate called') - .toBe(true); - done(); - }); - }); - -}); From ac59054cd451fcabbf75d914f98dd64212507b6b Mon Sep 17 00:00:00 2001 From: Andrew Scott Date: Thu, 2 Mar 2023 14:12:03 -0800 Subject: [PATCH 21/32] refactor(migrations): Move ChangeTracker to common utils (#49308) The `ChangeTracker` is generally useful and could be used by a lot of migrations instead of having to rewrite similar boilerplate. PR Close #49308 --- .../ng-generate/standalone-migration/index.ts | 3 +- .../standalone-migration/prune-modules.ts | 3 +- .../standalone-bootstrap.ts | 3 +- .../standalone-migration/to-standalone.ts | 3 +- .../ng-generate/standalone-migration/util.ts | 142 +--------------- .../core/schematics/utils/change_tracker.ts | 151 ++++++++++++++++++ 6 files changed, 160 insertions(+), 145 deletions(-) create mode 100644 packages/core/schematics/utils/change_tracker.ts diff --git a/packages/core/schematics/ng-generate/standalone-migration/index.ts b/packages/core/schematics/ng-generate/standalone-migration/index.ts index 12f23062f95d..63c9c728bccd 100644 --- a/packages/core/schematics/ng-generate/standalone-migration/index.ts +++ b/packages/core/schematics/ng-generate/standalone-migration/index.ts @@ -12,13 +12,14 @@ import {existsSync, statSync} from 'fs'; import {join, relative} from 'path'; import ts from 'typescript'; +import {ChangesByFile, normalizePath} from '../../utils/change_tracker'; import {getProjectTsConfigPaths} from '../../utils/project_tsconfig_paths'; import {canMigrateFile, createProgramOptions} from '../../utils/typescript/compiler_host'; import {pruneNgModules} from './prune-modules'; import {toStandaloneBootstrap} from './standalone-bootstrap'; import {toStandalone} from './to-standalone'; -import {ChangesByFile, knownInternalAliasRemapper, normalizePath} from './util'; +import {knownInternalAliasRemapper} from './util'; enum MigrationMode { toStandalone = 'convert-to-standalone', diff --git a/packages/core/schematics/ng-generate/standalone-migration/prune-modules.ts b/packages/core/schematics/ng-generate/standalone-migration/prune-modules.ts index 39447886404d..d7f21cdad35f 100644 --- a/packages/core/schematics/ng-generate/standalone-migration/prune-modules.ts +++ b/packages/core/schematics/ng-generate/standalone-migration/prune-modules.ts @@ -9,10 +9,11 @@ import {NgtscProgram} from '@angular/compiler-cli'; import ts from 'typescript'; +import {ChangeTracker, ImportRemapper} from '../../utils/change_tracker'; import {getAngularDecorators, NgDecorator} from '../../utils/ng_decorators'; import {closestNode} from '../../utils/typescript/nodes'; -import {ChangeTracker, findClassDeclaration, findLiteralProperty, getNodeLookup, ImportRemapper, offsetsToNodes, ReferenceResolver, UniqueItemTracker} from './util'; +import {findClassDeclaration, findLiteralProperty, getNodeLookup, offsetsToNodes, ReferenceResolver, UniqueItemTracker} from './util'; /** Keeps track of the places from which we need to remove AST nodes. */ interface RemovalLocations { diff --git a/packages/core/schematics/ng-generate/standalone-migration/standalone-bootstrap.ts b/packages/core/schematics/ng-generate/standalone-migration/standalone-bootstrap.ts index 0a8454596621..fae719f56c3a 100644 --- a/packages/core/schematics/ng-generate/standalone-migration/standalone-bootstrap.ts +++ b/packages/core/schematics/ng-generate/standalone-migration/standalone-bootstrap.ts @@ -12,11 +12,12 @@ import {TemplateTypeChecker} from '@angular/compiler-cli/private/migrations'; import {dirname, join} from 'path'; import ts from 'typescript'; +import {ChangeTracker, ImportRemapper} from '../../utils/change_tracker'; import {getAngularDecorators} from '../../utils/ng_decorators'; import {closestNode} from '../../utils/typescript/nodes'; import {ComponentImportsRemapper, convertNgModuleDeclarationToStandalone, extractDeclarationsFromModule, findTestObjectsToMigrate, migrateTestDeclarations} from './to-standalone'; -import {ChangeTracker, closestOrSelf, findClassDeclaration, findLiteralProperty, getNodeLookup, getRelativeImportPath, ImportRemapper, isClassReferenceInAngularModule, NamedClassDeclaration, NodeLookup, offsetsToNodes, ReferenceResolver, UniqueItemTracker} from './util'; +import {closestOrSelf, findClassDeclaration, findLiteralProperty, getNodeLookup, getRelativeImportPath, isClassReferenceInAngularModule, NamedClassDeclaration, NodeLookup, offsetsToNodes, ReferenceResolver, UniqueItemTracker} from './util'; /** Information extracted from a `bootstrapModule` call necessary to migrate it. */ interface BootstrapCallAnalysis { diff --git a/packages/core/schematics/ng-generate/standalone-migration/to-standalone.ts b/packages/core/schematics/ng-generate/standalone-migration/to-standalone.ts index b9add0068495..85dc38ce7236 100644 --- a/packages/core/schematics/ng-generate/standalone-migration/to-standalone.ts +++ b/packages/core/schematics/ng-generate/standalone-migration/to-standalone.ts @@ -10,12 +10,13 @@ import {NgtscProgram} from '@angular/compiler-cli'; import {PotentialImport, PotentialImportKind, PotentialImportMode, Reference, TemplateTypeChecker} from '@angular/compiler-cli/private/migrations'; import ts from 'typescript'; +import {ChangesByFile, ChangeTracker, ImportRemapper} from '../../utils/change_tracker'; import {getAngularDecorators, NgDecorator} from '../../utils/ng_decorators'; import {getImportSpecifier} from '../../utils/typescript/imports'; import {closestNode} from '../../utils/typescript/nodes'; import {isReferenceToImport} from '../../utils/typescript/symbol'; -import {ChangesByFile, ChangeTracker, findClassDeclaration, findLiteralProperty, ImportRemapper, isClassReferenceInAngularModule, NamedClassDeclaration} from './util'; +import {findClassDeclaration, findLiteralProperty, isClassReferenceInAngularModule, NamedClassDeclaration} from './util'; /** * Function that can be used to prcess the dependencies that diff --git a/packages/core/schematics/ng-generate/standalone-migration/util.ts b/packages/core/schematics/ng-generate/standalone-migration/util.ts index 1fa0fa16802a..d852381712fd 100644 --- a/packages/core/schematics/ng-generate/standalone-migration/util.ts +++ b/packages/core/schematics/ng-generate/standalone-migration/util.ts @@ -11,156 +11,21 @@ import {PotentialImport} from '@angular/compiler-cli/private/migrations'; import {dirname, relative} from 'path'; import ts from 'typescript'; -import {ImportManager} from '../../utils/import_manager'; +import {normalizePath} from '../../utils/change_tracker'; import {closestNode} from '../../utils/typescript/nodes'; -/** Mapping between a source file and the changes that have to be applied to it. */ -export type ChangesByFile = ReadonlyMap; - /** Map used to look up nodes based on their positions in a source file. */ export type NodeLookup = Map; /** Utility to type a class declaration with a name. */ export type NamedClassDeclaration = ts.ClassDeclaration&{name: ts.Identifier}; -/** Function that can be used to remap a generated import. */ -export type ImportRemapper = (moduleName: string, inFile: string) => string; - /** Text span of an AST node. */ export type ReferenceSpan = [start: number, end: number]; /** Mapping between a file name and spans for node references inside of it. */ export type ReferencesByFile = Map; -/** Change that needs to be applied to a file. */ -interface PendingChange { - /** Index at which to start changing the file. */ - start: number; - /** - * Amount of text that should be removed after the `start`. - * No text will be removed if omitted. - */ - removeLength?: number; - /** New text that should be inserted. */ - text: string; -} - -/** Tracks changes that have to be made for specific files. */ -export class ChangeTracker { - private readonly _changes = new Map(); - private readonly _importManager = new ImportManager( - currentFile => ({ - addNewImport: (start, text) => this.insertText(currentFile, start, text), - updateExistingImport: (namedBindings, text) => - this.replaceText(currentFile, namedBindings.getStart(), namedBindings.getWidth(), text), - }), - this._printer); - - constructor(private _printer: ts.Printer, private _importRemapper?: ImportRemapper) {} - - /** - * Tracks the insertion of some text. - * @param sourceFile File in which the text is being inserted. - * @param start Index at which the text is insert. - * @param text Text to be inserted. - */ - insertText(sourceFile: ts.SourceFile, index: number, text: string): void { - this._trackChange(sourceFile, {start: index, text}); - } - - /** - * Replaces text within a file. - * @param sourceFile File in which to replace the text. - * @param start Index from which to replace the text. - * @param removeLength Length of the text being replaced. - * @param text Text to be inserted instead of the old one. - */ - replaceText(sourceFile: ts.SourceFile, start: number, removeLength: number, text: string): void { - this._trackChange(sourceFile, {start, removeLength, text}); - } - - /** - * Replaces the text of an AST node with a new one. - * @param oldNode Node to be replaced. - * @param newNode New node to be inserted. - * @param emitHint Hint when formatting the text of the new node. - * @param sourceFileWhenPrinting File to use when printing out the new node. This is important - * when copying nodes from one file to another, because TypeScript might not output literal nodes - * without it. - */ - replaceNode( - oldNode: ts.Node, newNode: ts.Node, emitHint = ts.EmitHint.Unspecified, - sourceFileWhenPrinting?: ts.SourceFile): void { - const sourceFile = oldNode.getSourceFile(); - this.replaceText( - sourceFile, oldNode.getStart(), oldNode.getWidth(), - this._printer.printNode(emitHint, newNode, sourceFileWhenPrinting || sourceFile)); - } - - /** - * Removes the text of an AST node from a file. - * @param node Node whose text should be removed. - */ - removeNode(node: ts.Node): void { - this._trackChange( - node.getSourceFile(), {start: node.getStart(), removeLength: node.getWidth(), text: ''}); - } - - /** - * Adds an import to a file. - * @param sourceFile File to which to add the import. - * @param symbolName Symbol being imported. - * @param moduleName Module from which the symbol is imported. - */ - addImport( - sourceFile: ts.SourceFile, symbolName: string, moduleName: string, - alias: string|null = null): ts.Expression { - if (this._importRemapper) { - moduleName = this._importRemapper(moduleName, sourceFile.fileName); - } - - // It's common for paths to be manipulated with Node's `path` utilties which - // can yield a path with back slashes. Normalize them since outputting such - // paths will also cause TS to escape the forward slashes. - moduleName = normalizePath(moduleName); - - return this._importManager.addImportToSourceFile(sourceFile, symbolName, moduleName, alias); - } - - /** - * Gets the changes that should be applied to all the files in the migration. - * The changes are sorted in the order in which they should be applied. - */ - recordChanges(): ChangesByFile { - this._importManager.recordChanges(); - return this._changes; - } - - /** - * Adds a change to a `ChangesByFile` map. - * @param file File that the change is associated with. - * @param change Change to be added. - */ - private _trackChange(file: ts.SourceFile, change: PendingChange): void { - const changes = this._changes.get(file); - - if (changes) { - // Insert the changes in reverse so that they're applied in reverse order. - // This ensures that the offsets of subsequent changes aren't affected by - // previous changes changing the file's text. - const insertIndex = changes.findIndex(current => current.start <= change.start); - - if (insertIndex === -1) { - changes.push(change); - } else { - changes.splice(insertIndex, 0, change); - } - } else { - this._changes.set(file, [change]); - } - } -} - /** Utility class used to track a one-to-many relationship where all the items are unique. */ export class UniqueItemTracker { private _nodes = new Map>(); @@ -384,11 +249,6 @@ export function getRelativeImportPath(fromFile: string, toFile: string): string return normalizePath(path); } -/** Normalizes a path to use posix separators. */ -export function normalizePath(path: string): string { - return path.replace(/\\/g, '/'); -} - /** Function used to remap the generated `imports` for a component to known shorter aliases. */ export function knownInternalAliasRemapper(imports: PotentialImport[]) { return imports.map( diff --git a/packages/core/schematics/utils/change_tracker.ts b/packages/core/schematics/utils/change_tracker.ts new file mode 100644 index 000000000000..a3364c310a1d --- /dev/null +++ b/packages/core/schematics/utils/change_tracker.ts @@ -0,0 +1,151 @@ +/** + * @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 ts from 'typescript'; + +import {ImportManager} from './import_manager'; + +/** Function that can be used to remap a generated import. */ +export type ImportRemapper = (moduleName: string, inFile: string) => string; + +/** Mapping between a source file and the changes that have to be applied to it. */ +export type ChangesByFile = ReadonlyMap; + +/** Change that needs to be applied to a file. */ +export interface PendingChange { + /** Index at which to start changing the file. */ + start: number; + /** + * Amount of text that should be removed after the `start`. + * No text will be removed if omitted. + */ + removeLength?: number; + /** New text that should be inserted. */ + text: string; +} + +/** Tracks changes that have to be made for specific files. */ +export class ChangeTracker { + private readonly _changes = new Map(); + private readonly _importManager = new ImportManager( + currentFile => ({ + addNewImport: (start, text) => this.insertText(currentFile, start, text), + updateExistingImport: (namedBindings, text) => + this.replaceText(currentFile, namedBindings.getStart(), namedBindings.getWidth(), text), + }), + this._printer); + + constructor(private _printer: ts.Printer, private _importRemapper?: ImportRemapper) {} + + /** + * Tracks the insertion of some text. + * @param sourceFile File in which the text is being inserted. + * @param start Index at which the text is insert. + * @param text Text to be inserted. + */ + insertText(sourceFile: ts.SourceFile, index: number, text: string): void { + this._trackChange(sourceFile, {start: index, text}); + } + + /** + * Replaces text within a file. + * @param sourceFile File in which to replace the text. + * @param start Index from which to replace the text. + * @param removeLength Length of the text being replaced. + * @param text Text to be inserted instead of the old one. + */ + replaceText(sourceFile: ts.SourceFile, start: number, removeLength: number, text: string): void { + this._trackChange(sourceFile, {start, removeLength, text}); + } + + /** + * Replaces the text of an AST node with a new one. + * @param oldNode Node to be replaced. + * @param newNode New node to be inserted. + * @param emitHint Hint when formatting the text of the new node. + * @param sourceFileWhenPrinting File to use when printing out the new node. This is important + * when copying nodes from one file to another, because TypeScript might not output literal nodes + * without it. + */ + replaceNode( + oldNode: ts.Node, newNode: ts.Node, emitHint = ts.EmitHint.Unspecified, + sourceFileWhenPrinting?: ts.SourceFile): void { + const sourceFile = oldNode.getSourceFile(); + this.replaceText( + sourceFile, oldNode.getStart(), oldNode.getWidth(), + this._printer.printNode(emitHint, newNode, sourceFileWhenPrinting || sourceFile)); + } + + /** + * Removes the text of an AST node from a file. + * @param node Node whose text should be removed. + */ + removeNode(node: ts.Node): void { + this._trackChange( + node.getSourceFile(), {start: node.getStart(), removeLength: node.getWidth(), text: ''}); + } + + /** + * Adds an import to a file. + * @param sourceFile File to which to add the import. + * @param symbolName Symbol being imported. + * @param moduleName Module from which the symbol is imported. + */ + addImport( + sourceFile: ts.SourceFile, symbolName: string, moduleName: string, + alias: string|null = null): ts.Expression { + if (this._importRemapper) { + moduleName = this._importRemapper(moduleName, sourceFile.fileName); + } + + // It's common for paths to be manipulated with Node's `path` utilties which + // can yield a path with back slashes. Normalize them since outputting such + // paths will also cause TS to escape the forward slashes. + moduleName = normalizePath(moduleName); + + return this._importManager.addImportToSourceFile(sourceFile, symbolName, moduleName, alias); + } + + /** + * Gets the changes that should be applied to all the files in the migration. + * The changes are sorted in the order in which they should be applied. + */ + recordChanges(): ChangesByFile { + this._importManager.recordChanges(); + return this._changes; + } + + /** + * Adds a change to a `ChangesByFile` map. + * @param file File that the change is associated with. + * @param change Change to be added. + */ + private _trackChange(file: ts.SourceFile, change: PendingChange): void { + const changes = this._changes.get(file); + + if (changes) { + // Insert the changes in reverse so that they're applied in reverse order. + // This ensures that the offsets of subsequent changes aren't affected by + // previous changes changing the file's text. + const insertIndex = changes.findIndex(current => current.start <= change.start); + + if (insertIndex === -1) { + changes.push(change); + } else { + changes.splice(insertIndex, 0, change); + } + } else { + this._changes.set(file, [change]); + } + } +} + +/** Normalizes a path to use posix separators. */ +export function normalizePath(path: string): string { + return path.replace(/\\/g, '/'); +} From 182258520dfac2d97a4b88dc16b973a5e9b81f65 Mon Sep 17 00:00:00 2001 From: Matthieu Riegler Date: Mon, 27 Feb 2023 19:36:39 +0100 Subject: [PATCH 22/32] refactor(platform-browser): handle #24571 todos (#49232) This commit removes the remaining TODO(issue/24571) in platform-browser code base. PR Close #49232 --- packages/platform-browser/src/dom/events/event_manager.ts | 6 ++++-- packages/platform-browser/test/testing_public_spec.ts | 4 +--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/platform-browser/src/dom/events/event_manager.ts b/packages/platform-browser/src/dom/events/event_manager.ts index c8cfa17fc332..f19ace737dc6 100644 --- a/packages/platform-browser/src/dom/events/event_manager.ts +++ b/packages/platform-browser/src/dom/events/event_manager.ts @@ -32,7 +32,9 @@ export class EventManager { * Initializes an instance of the event-manager service. */ constructor(@Inject(EVENT_MANAGER_PLUGINS) plugins: EventManagerPlugin[], private _zone: NgZone) { - plugins.forEach(p => p.manager = this); + plugins.forEach((plugin) => { + plugin.manager = this; + }); this._plugins = plugins.slice().reverse(); } @@ -94,7 +96,7 @@ export class EventManager { export abstract class EventManagerPlugin { constructor(private _doc: any) {} - // TODO(issue/24571): remove '!'. + // Using non-null assertion because it's set by EventManager's constructor manager!: EventManager; abstract supports(eventName: string): boolean; diff --git a/packages/platform-browser/test/testing_public_spec.ts b/packages/platform-browser/test/testing_public_spec.ts index 00fb979d43e1..71636f07d253 100644 --- a/packages/platform-browser/test/testing_public_spec.ts +++ b/packages/platform-browser/test/testing_public_spec.ts @@ -7,7 +7,7 @@ */ import {ResourceLoader} from '@angular/compiler'; -import {Compiler, Component, ComponentFactoryResolver, CUSTOM_ELEMENTS_SCHEMA, Directive, Inject, Injectable, InjectionToken, Injector, Input, NgModule, Optional, Pipe, SkipSelf, Type, ɵstringify as stringify} from '@angular/core'; +import {Compiler, Component, ComponentFactoryResolver, CUSTOM_ELEMENTS_SCHEMA, Directive, Inject, Injectable, InjectionToken, Injector, Input, NgModule, Optional, Pipe, SkipSelf, Type} from '@angular/core'; import {fakeAsync, getTestBed, inject, TestBed, tick, waitForAsync, withModule} from '@angular/core/testing'; import {expect} from '@angular/platform-browser/testing/src/matchers'; @@ -92,7 +92,6 @@ class TestViewProvidersComp { @Directive({selector: '[someDir]', host: {'[title]': 'someDir'}}) class SomeDirective { - // TODO(issue/24571): remove '!'. @Input() someDir!: string; } @@ -762,7 +761,6 @@ const bTok = new InjectionToken('b'); testDir = this; } - // TODO(issue/24571): remove '!'. @Input('test') test!: string; } From 980147555c7bbe1516fcee92abae5a7dc576fd91 Mon Sep 17 00:00:00 2001 From: Andrew Kushnir Date: Fri, 3 Mar 2023 19:14:42 -0800 Subject: [PATCH 23/32] refactor(core): rename TNode.tViews to TNode.tView (#49313) Previously (at the early days of Ivy) a TNode used to keep an array of TViews, but the logic was changed since that time, but the `tViews` field remained on TNode interface (+ corresponding typings). This commit renames TNode.tViews to TNode.tView and cleans up typings. PR Close #49313 --- packages/core/src/linker/template_ref.ts | 4 +-- packages/core/src/render3/i18n/i18n_util.ts | 5 ++-- .../core/src/render3/instructions/shared.ts | 5 ++-- .../core/src/render3/instructions/template.ts | 3 +- packages/core/src/render3/interfaces/node.ts | 30 ++++++------------- .../core/test/render3/instructions_spec.ts | 2 +- packages/core/test/render3/is_shape_of.ts | 2 +- 7 files changed, 20 insertions(+), 31 deletions(-) diff --git a/packages/core/src/linker/template_ref.ts b/packages/core/src/linker/template_ref.ts index 389863c15095..b2ff10c68eff 100644 --- a/packages/core/src/linker/template_ref.ts +++ b/packages/core/src/linker/template_ref.ts @@ -79,7 +79,7 @@ const R3TemplateRef = class TemplateRef extends ViewEngineTemplateRef { } override createEmbeddedView(context: T, injector?: Injector): EmbeddedViewRef { - const embeddedTView = this._declarationTContainer.tViews as TView; + const embeddedTView = this._declarationTContainer.tView as TView; const embeddedLView = createLView( this._declarationLView, embeddedTView, context, LViewFlags.CheckAlways, null, embeddedTView.declTNode, null, null, null, null, injector || null); @@ -117,7 +117,7 @@ export function injectTemplateRef(): TemplateRef|null { */ export function createTemplateRef(hostTNode: TNode, hostLView: LView): TemplateRef|null { if (hostTNode.type & TNodeType.Container) { - ngDevMode && assertDefined(hostTNode.tViews, 'TView must be allocated'); + ngDevMode && assertDefined(hostTNode.tView, 'TView must be allocated'); return new R3TemplateRef( hostLView, hostTNode as TContainerNode, createElementRef(hostTNode, hostLView)); } diff --git a/packages/core/src/render3/i18n/i18n_util.ts b/packages/core/src/render3/i18n/i18n_util.ts index df3e2b5cc0b5..8c5a646290e3 100644 --- a/packages/core/src/render3/i18n/i18n_util.ts +++ b/packages/core/src/render3/i18n/i18n_util.ts @@ -15,6 +15,7 @@ import {LView, TView} from '../interfaces/view'; import {assertTNodeType} from '../node_assert'; import {setI18nHandling} from '../node_manipulation'; import {getInsertInFrontOfRNodeWithI18n, processI18nInsertBefore} from '../node_manipulation_i18n'; + import {addTNodeAndUpdateInsertBeforeIndex} from './i18n_insert_before_index'; @@ -35,7 +36,7 @@ export function getTIcu(tView: TView, index: number): TIcu|null { const value = tView.data[index] as null | TIcu | TIcuContainerNode | string; if (value === null || typeof value === 'string') return null; if (ngDevMode && - !(value.hasOwnProperty('tViews') || value.hasOwnProperty('currentCaseLViewIndex'))) { + !(value.hasOwnProperty('tView') || value.hasOwnProperty('currentCaseLViewIndex'))) { throwError('We expect to get \'null\'|\'TIcu\'|\'TIcuContainer\', but got: ' + value); } // Here the `value.hasOwnProperty('currentCaseLViewIndex')` is a polymorphic read as it can be @@ -66,7 +67,7 @@ export function setTIcu(tView: TView, index: number, tIcu: TIcu): void { const tNode = tView.data[index] as null | TIcuContainerNode; ngDevMode && assertEqual( - tNode === null || tNode.hasOwnProperty('tViews'), true, + tNode === null || tNode.hasOwnProperty('tView'), true, 'We expect to get \'null\'|\'TIcuContainer\''); if (tNode === null) { tView.data[index] = tIcu; diff --git a/packages/core/src/render3/instructions/shared.ts b/packages/core/src/render3/instructions/shared.ts index 96b19703d384..1c1444a8b14d 100644 --- a/packages/core/src/render3/instructions/shared.ts +++ b/packages/core/src/render3/instructions/shared.ts @@ -702,13 +702,12 @@ export function storeCleanupWithContext( /** * Constructs a TNode object from the arguments. * - * @param tView `TView` to which this `TNode` belongs (used only in `ngDevMode`) + * @param tView `TView` to which this `TNode` belongs * @param tParent Parent `TNode` * @param type The type of the node * @param index The index of the TNode in TView.data, adjusted for HEADER_OFFSET * @param tagName The tag name of the node * @param attrs The attributes defined on this node - * @param tViews Any TViews attached to this node * @returns the TNode object */ export function createTNode( @@ -758,7 +757,7 @@ export function createTNode( initialInputs: undefined, inputs: null, outputs: null, - tViews: null, + tView: null, next: null, prev: null, projectionNext: null, diff --git a/packages/core/src/render3/instructions/template.ts b/packages/core/src/render3/instructions/template.ts index 43e476ca6126..baca780732ef 100644 --- a/packages/core/src/render3/instructions/template.ts +++ b/packages/core/src/render3/instructions/template.ts @@ -15,6 +15,7 @@ import {HEADER_OFFSET, LView, RENDERER, TView, TViewType} from '../interfaces/vi import {appendChild} from '../node_manipulation'; import {getLView, getTView, setCurrentTNode} from '../state'; import {getConstant} from '../util/view_utils'; + import {addToViewTree, createDirectivesInstances, createLContainer, createTView, getOrCreateTNode, resolveDirectives, saveResolvedLocalsInData} from './shared'; @@ -34,7 +35,7 @@ function templateFirstCreatePass( resolveDirectives(tView, lView, tNode, getConstant(tViewConsts, localRefsIndex)); registerPostOrderHooks(tView, tNode); - const embeddedTView = tNode.tViews = createTView( + const embeddedTView = tNode.tView = createTView( TViewType.Embedded, tNode, templateFn, decls, vars, tView.directiveRegistry, tView.pipeRegistry, null, tView.schemas, tViewConsts); diff --git a/packages/core/src/render3/interfaces/node.ts b/packages/core/src/render3/interfaces/node.ts index 25c2f10127d5..eaa156681560 100644 --- a/packages/core/src/render3/interfaces/node.ts +++ b/packages/core/src/render3/interfaces/node.ts @@ -532,26 +532,14 @@ export interface TNode { outputs: PropertyAliases|null; /** - * The TView or TViews attached to this node. - * - * If this TNode corresponds to an LContainer with inline views, the container will - * need to store separate static data for each of its view blocks (TView[]). Otherwise, - * nodes in inline views with the same index as nodes in their parent views will overwrite - * each other, as they are in the same template. - * - * Each index in this array corresponds to the static data for a certain - * view. So if you had V(0) and V(1) in a container, you might have: - * - * [ - * [{tagName: 'div', attrs: ...}, null], // V(0) TView - * [{tagName: 'button', attrs ...}, null] // V(1) TView + * The TView attached to this node. * * If this TNode corresponds to an LContainer with a template (e.g. structural * directive), the template's TView will be stored here. * - * If this TNode corresponds to an element, tViews will be null . + * If this TNode corresponds to an element, tView will be `null`. */ - tViews: TView|TView[]|null; + tView: TView|null; /** * The next sibling node. Necessary so we can propagate through the root nodes of a view @@ -774,7 +762,7 @@ export interface TElementNode extends TNode { * retrieved using viewData[HOST_NODE]). */ parent: TElementNode|TElementContainerNode|null; - tViews: null; + tView: null; /** * If this is a component TNode with projection, this will be an array of projected @@ -800,7 +788,7 @@ export interface TTextNode extends TNode { * retrieved using LView.node). */ parent: TElementNode|TElementContainerNode|null; - tViews: null; + tView: null; projection: null; } @@ -822,7 +810,7 @@ export interface TContainerNode extends TNode { * - They are dynamically created */ parent: TElementNode|TElementContainerNode|null; - tViews: TView|TView[]|null; + tView: TView|null; projection: null; value: null; } @@ -833,7 +821,7 @@ export interface TElementContainerNode extends TNode { index: number; child: TElementNode|TTextNode|TContainerNode|TElementContainerNode|TProjectionNode|null; parent: TElementNode|TElementContainerNode|null; - tViews: null; + tView: null; projection: null; } @@ -843,7 +831,7 @@ export interface TIcuContainerNode extends TNode { index: number; child: null; parent: TElementNode|TElementContainerNode|null; - tViews: null; + tView: null; projection: null; value: TIcu; } @@ -858,7 +846,7 @@ export interface TProjectionNode extends TNode { * retrieved using LView.node). */ parent: TElementNode|TElementContainerNode|null; - tViews: null; + tView: null; /** Index of the projection node. (See TNode.projection for more info.) */ projection: number; diff --git a/packages/core/test/render3/instructions_spec.ts b/packages/core/test/render3/instructions_spec.ts index 04b4e0bce953..fc165cc60fb2 100644 --- a/packages/core/test/render3/instructions_spec.ts +++ b/packages/core/test/render3/instructions_spec.ts @@ -282,7 +282,7 @@ describe('instructions', () => { }); describe('performance counters', () => { - it('should create tViews only once for each nested level', () => { + it('should create tView only once for each nested level', () => { @Component({ selector: 'nested-loops', standalone: true, diff --git a/packages/core/test/render3/is_shape_of.ts b/packages/core/test/render3/is_shape_of.ts index b40b7dbdd9e8..c09ade9d5c98 100644 --- a/packages/core/test/render3/is_shape_of.ts +++ b/packages/core/test/render3/is_shape_of.ts @@ -165,7 +165,7 @@ const ShapeOfTNode: ShapeOf = { initialInputs: true, inputs: true, outputs: true, - tViews: true, + tView: true, next: true, prev: true, projectionNext: true, From 84f45363eb3cf0bc48173b3f963cb509db3d5a08 Mon Sep 17 00:00:00 2001 From: Matthieu Riegler Date: Thu, 2 Mar 2023 22:48:05 +0100 Subject: [PATCH 24/32] refactor(platform-browser): remove ununsed functions. (#49302) Both `camelCaseToDashCase` and `dashCaseToCamelCase` haven't been used since 2.2.0. PR Close #49302 --- packages/platform-browser/src/dom/util.ts | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/packages/platform-browser/src/dom/util.ts b/packages/platform-browser/src/dom/util.ts index f4c36de9aaf0..66d737843c45 100644 --- a/packages/platform-browser/src/dom/util.ts +++ b/packages/platform-browser/src/dom/util.ts @@ -8,18 +8,6 @@ import {ɵglobal as global} from '@angular/core'; -const CAMEL_CASE_REGEXP = /([A-Z])/g; -const DASH_CASE_REGEXP = /-([a-z])/g; - - -export function camelCaseToDashCase(input: string): string { - return input.replace(CAMEL_CASE_REGEXP, (...m: string[]) => '-' + m[1].toLowerCase()); -} - -export function dashCaseToCamelCase(input: string): string { - return input.replace(DASH_CASE_REGEXP, (...m: string[]) => m[1].toUpperCase()); -} - /** * Exports the value under a given `name` in the global property `ng`. For example `ng.probe` if * `name` is `'probe'`. From b13e079bc70009b8d36b31626e4b0012fb988112 Mon Sep 17 00:00:00 2001 From: Matthieu Riegler Date: Thu, 2 Mar 2023 20:34:58 +0100 Subject: [PATCH 25/32] refactor(core): Remove `ChangeDetectorStatus` & `isDefaultChangeDetectionStrategy` (#49299) Unused code in core and other packages. They were private exports. PR Close #49299 --- .../src/change_detection/change_detection.ts | 2 +- .../core/src/change_detection/constants.ts | 55 ------------------- packages/core/src/core_private_export.ts | 1 - 3 files changed, 1 insertion(+), 57 deletions(-) diff --git a/packages/core/src/change_detection/change_detection.ts b/packages/core/src/change_detection/change_detection.ts index aa90a847c5cd..1f8ae89f0dcb 100644 --- a/packages/core/src/change_detection/change_detection.ts +++ b/packages/core/src/change_detection/change_detection.ts @@ -14,7 +14,7 @@ import {KeyValueDifferFactory, KeyValueDiffers} from './differs/keyvalue_differs export {SimpleChange, SimpleChanges} from '../interface/simple_change'; export {devModeEqual} from '../util/comparison'; export {ChangeDetectorRef} from './change_detector_ref'; -export {ChangeDetectionStrategy, ChangeDetectorStatus, isDefaultChangeDetectionStrategy} from './constants'; +export {ChangeDetectionStrategy} from './constants'; export {DefaultIterableDiffer, DefaultIterableDifferFactory} from './differs/default_iterable_differ'; export {DefaultKeyValueDifferFactory} from './differs/default_keyvalue_differ'; export {IterableChangeRecord, IterableChanges, IterableDiffer, IterableDifferFactory, IterableDiffers, NgIterable, TrackByFunction} from './differs/iterable_differs'; diff --git a/packages/core/src/change_detection/constants.ts b/packages/core/src/change_detection/constants.ts index bc81f02f627d..c8a53ab8c7a5 100644 --- a/packages/core/src/change_detection/constants.ts +++ b/packages/core/src/change_detection/constants.ts @@ -30,58 +30,3 @@ export enum ChangeDetectionStrategy { */ Default = 1, } - -/** - * Defines the possible states of the default change detector. - * @see `ChangeDetectorRef` - */ -export enum ChangeDetectorStatus { - /** - * A state in which, after calling `detectChanges()`, the change detector - * state becomes `Checked`, and must be explicitly invoked or reactivated. - */ - CheckOnce, - - /** - * A state in which change detection is skipped until the change detector mode - * becomes `CheckOnce`. - */ - Checked, - - /** - * A state in which change detection continues automatically until explicitly - * deactivated. - */ - CheckAlways, - - /** - * A state in which a change detector sub tree is not a part of the main tree and - * should be skipped. - */ - Detached, - - /** - * Indicates that the change detector encountered an error checking a binding - * or calling a directive lifecycle method and is now in an inconsistent state. Change - * detectors in this state do not detect changes. - */ - Errored, - - /** - * Indicates that the change detector has been destroyed. - */ - Destroyed, -} - -/** - * Reports whether a given strategy is currently the default for change detection. - * @param changeDetectionStrategy The strategy to check. - * @returns True if the given strategy is the current default, false otherwise. - * @see `ChangeDetectorStatus` - * @see `ChangeDetectorRef` - */ -export function isDefaultChangeDetectionStrategy(changeDetectionStrategy: ChangeDetectionStrategy): - boolean { - return changeDetectionStrategy == null || - changeDetectionStrategy === ChangeDetectionStrategy.Default; -} diff --git a/packages/core/src/core_private_export.ts b/packages/core/src/core_private_export.ts index 517aa08bf9f5..dddc6b97de77 100644 --- a/packages/core/src/core_private_export.ts +++ b/packages/core/src/core_private_export.ts @@ -9,7 +9,6 @@ export {ALLOW_MULTIPLE_PLATFORMS as ɵALLOW_MULTIPLE_PLATFORMS, internalCreateApplication as ɵinternalCreateApplication} from './application_ref'; export {APP_ID_RANDOM_PROVIDER as ɵAPP_ID_RANDOM_PROVIDER} from './application_tokens'; export {defaultIterableDiffers as ɵdefaultIterableDiffers, defaultKeyValueDiffers as ɵdefaultKeyValueDiffers} from './change_detection/change_detection'; -export {ChangeDetectorStatus as ɵChangeDetectorStatus, isDefaultChangeDetectionStrategy as ɵisDefaultChangeDetectionStrategy} from './change_detection/constants'; export {Console as ɵConsole} from './console'; export {getDebugNodeR2 as ɵgetDebugNodeR2} from './debug/debug_node'; export {convertToBitFlags as ɵconvertToBitFlags, setCurrentInjector as ɵsetCurrentInjector} from './di/injector_compatibility'; From ffccb8e3a7595e9e37c70c53c402a96b3dc523d8 Mon Sep 17 00:00:00 2001 From: Matthieu Riegler Date: Mon, 27 Feb 2023 19:33:12 +0100 Subject: [PATCH 26/32] refactor(animations): handle #24571 todos (#49231) This commit removes the remaining ones in animations code base PR Close #49231 --- .../browser/src/dsl/animation_timeline_builder.ts | 3 +-- .../browser/src/render/transition_animation_engine.ts | 3 +-- .../src/render/web_animations/web_animations_player.ts | 6 +++--- 3 files changed, 5 insertions(+), 7 deletions(-) diff --git a/packages/animations/browser/src/dsl/animation_timeline_builder.ts b/packages/animations/browser/src/dsl/animation_timeline_builder.ts index e4e9b90bddea..b20800e1ebca 100644 --- a/packages/animations/browser/src/dsl/animation_timeline_builder.ts +++ b/packages/animations/browser/src/dsl/animation_timeline_builder.ts @@ -615,8 +615,7 @@ export class AnimationTimelineContext { export class TimelineBuilder { public duration: number = 0; - // TODO(issue/24571): remove '!'. - public easing!: string|null; + public easing: string|null = null; private _previousKeyframe: ɵStyleDataMap = new Map(); private _currentKeyframe: ɵStyleDataMap = new Map(); private _keyframes = new Map(); diff --git a/packages/animations/browser/src/render/transition_animation_engine.ts b/packages/animations/browser/src/render/transition_animation_engine.ts index 9bbd8f5ee71b..eee66263a163 100644 --- a/packages/animations/browser/src/render/transition_animation_engine.ts +++ b/packages/animations/browser/src/render/transition_animation_engine.ts @@ -1499,8 +1499,7 @@ export class TransitionAnimationPlayer implements AnimationPlayer { private _queuedCallbacks = new Map any)[]>(); public readonly destroyed = false; - // TODO(issue/24571): remove '!'. - public parentPlayer!: AnimationPlayer; + public parentPlayer: AnimationPlayer|null = null; public markedForDestroy: boolean = false; public disabled = false; diff --git a/packages/animations/browser/src/render/web_animations/web_animations_player.ts b/packages/animations/browser/src/render/web_animations/web_animations_player.ts index 59d201eb08e9..bf772744a815 100644 --- a/packages/animations/browser/src/render/web_animations/web_animations_player.ts +++ b/packages/animations/browser/src/render/web_animations/web_animations_player.ts @@ -30,7 +30,7 @@ export class WebAnimationsPlayer implements AnimationPlayer { private _originalOnDoneFns: Function[] = []; private _originalOnStartFns: Function[] = []; - // TODO(issue/24571): remove '!'. + // using non-null assertion because it's re(set) by init(); public readonly domPlayer!: DOMAnimation; public time = 0; @@ -64,8 +64,8 @@ export class WebAnimationsPlayer implements AnimationPlayer { this._initialized = true; const keyframes = this.keyframes; - (this as {domPlayer: DOMAnimation}).domPlayer = - this._triggerWebAnimation(this.element, keyframes, this.options); + // @ts-expect-error overwriting a readonly property + this.domPlayer = this._triggerWebAnimation(this.element, keyframes, this.options); this._finalKeyframe = keyframes.length ? keyframes[keyframes.length - 1] : new Map(); this.domPlayer.addEventListener('finish', () => this._onFinish()); } From e0be553abf2db75d4eae2b8b562f98369948b95e Mon Sep 17 00:00:00 2001 From: Matthieu Riegler Date: Mon, 6 Mar 2023 22:41:50 +0100 Subject: [PATCH 27/32] docs(forms): change error code in filename to match enum value. (#49344) Error code for bad AsyncValidatorFn is 1101 not 1003. PR Close #49344 --- aio/content/errors/{NG01003.md => NG01101.md} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename aio/content/errors/{NG01003.md => NG01101.md} (100%) diff --git a/aio/content/errors/NG01003.md b/aio/content/errors/NG01101.md similarity index 100% rename from aio/content/errors/NG01003.md rename to aio/content/errors/NG01101.md From d0a73b24fedb61e42de41077cb6e382b6238ff14 Mon Sep 17 00:00:00 2001 From: Vinit Neogi <20491952+vneogi199@users.noreply.github.com> Date: Tue, 7 Mar 2023 22:55:25 +0530 Subject: [PATCH 28/32] docs(docs-infra): missing "is" in glossary (#49356) documentation glossary for view hierarchy is missing a "is" to be grammatically correct Fixes #49352 PR Close #49356 --- aio/content/guide/glossary.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/aio/content/guide/glossary.md b/aio/content/guide/glossary.md index abcb478e15ed..4d98b326dba8 100644 --- a/aio/content/guide/glossary.md +++ b/aio/content/guide/glossary.md @@ -929,7 +929,7 @@ View Engine was deprecated in version 9 and removed in version 13. ## view hierarchy A tree of related views that can be acted on as a unit. -The root view referenced as the *host view* of a component. +The root view is referenced as the *host view* of a component. A host view is the root of a tree of *embedded views*, collected in a `ViewContainerRef` view container attached to an anchor element in the hosting component. The view hierarchy is a key part of Angular [change detection][AioGuideGlossaryChangeDetection]. From 75b2a265a99a6d43e89aa39040329ca744179286 Mon Sep 17 00:00:00 2001 From: Sai Kartheek Bommisetty Date: Fri, 3 Mar 2023 18:00:36 +0530 Subject: [PATCH 29/32] docs: update toh-pt5.md (#49307) PR Close #49307 --- aio/content/tutorial/tour-of-heroes/toh-pt5.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/aio/content/tutorial/tour-of-heroes/toh-pt5.md b/aio/content/tutorial/tour-of-heroes/toh-pt5.md index 806de68a54d7..67275f59ce5f 100644 --- a/aio/content/tutorial/tour-of-heroes/toh-pt5.md +++ b/aio/content/tutorial/tour-of-heroes/toh-pt5.md @@ -206,7 +206,7 @@ This `getHeroes()` returns the sliced list of heroes at positions 1 and 5, retur To navigate to the dashboard, the router needs an appropriate route. -Import the `DashboardComponent` in the `app-routing-module.ts` file. +Import the `DashboardComponent` in the `app-routing.module.ts` file. From 88a0ff74adece61674da25775978a425ed17f6a5 Mon Sep 17 00:00:00 2001 From: Matthieu Riegler Date: Mon, 6 Mar 2023 22:15:05 +0100 Subject: [PATCH 30/32] docs(docs-infra): Split Error by categories & sort by code. (#49343) * 2 categories : Runtime errors and Compiler Errors. * Numeric sort on error code. PR Close #49343 --- .../processors/processErrorsContainerDoc.js | 29 +++++++++++++++-- .../error/errors-container.template.html | 31 +++++++++---------- 2 files changed, 41 insertions(+), 19 deletions(-) diff --git a/aio/tools/transforms/angular-errors-package/processors/processErrorsContainerDoc.js b/aio/tools/transforms/angular-errors-package/processors/processErrorsContainerDoc.js index 876674beff8a..a52dccf59940 100644 --- a/aio/tools/transforms/angular-errors-package/processors/processErrorsContainerDoc.js +++ b/aio/tools/transforms/angular-errors-package/processors/processErrorsContainerDoc.js @@ -3,9 +3,32 @@ module.exports = function processErrorsContainerDoc() { $runAfter: ['extra-docs-added'], $runBefore: ['rendering-docs'], $process(docs) { - const errorsDoc = docs.find(doc => doc.id === 'errors/index'); + const errorsDoc = docs.find((doc) => doc.id === 'errors/index'); errorsDoc.id = 'errors-container'; - errorsDoc.errors = docs.filter(doc => doc.docType === 'error'); - } + errorsDoc.errors = [ + { + title: 'Runtime', + errors: docs + .filter((doc) => doc.docType === 'error' && doc.category === 'runtime') + .sort(byCode), + }, + { + title: 'Compiler', + errors: docs + .filter((doc) => doc.docType === 'error' && doc.category === 'compiler') + .sort(byCode), + }, + ]; + }, }; }; + +/** + * Helper function to sort documents by error codes (NG0100 will come before NG0200) + */ +function byCode(doc1, doc2) { + // slice to drop the 'NG' part of the code. + const code1 = +doc1.code.slice(2); + const code2 = +doc2.code.slice(2); + return code1 > code2 ? 1 : -1; +} diff --git a/aio/tools/transforms/templates/error/errors-container.template.html b/aio/tools/transforms/templates/error/errors-container.template.html index 606a9f671621..18eacec782dc 100644 --- a/aio/tools/transforms/templates/error/errors-container.template.html +++ b/aio/tools/transforms/templates/error/errors-container.template.html @@ -1,18 +1,17 @@ -{% extends 'content.template.html' -%} +{% extends 'content.template.html' -%} {% block content %} +
{$ doc.description | marked $}
-{% block content %} -
- {$ doc.description | marked $} -
- - -{% endblock %} \ No newline at end of file +{% endblock %} From 993750815ec284af528c3c24eb4739b7dfef0675 Mon Sep 17 00:00:00 2001 From: Paul Gschwendtner Date: Tue, 7 Mar 2023 10:30:23 +0000 Subject: [PATCH 31/32] refactor(compiler-cli): remove unused class decorator downlevel code (#49351) The decorator downlevel transform is never used for actual class decorators because Angular class decorators rely on immediate execution for JIT. Initially we also supported downleveling of class decorators for View Engine library output, but libraries are shipped using partial compilation output and are not using this transform anymore. The transform is exclusively used for JIT processing, commonly for test files to help ease temporal dead-zone/forward-ref issues. We can remove the class decorator downlevel logic to remove technical debt. PR Close #49351 --- packages/compiler-cli/private/tooling.ts | 2 +- .../downlevel_decorators_transform.ts | 78 ++---- .../downlevel_decorators_transform_spec.ts | 232 +++++------------- .../downlevel_decorator_transform.ts | 2 +- yarn.lock | 14 +- 5 files changed, 91 insertions(+), 237 deletions(-) diff --git a/packages/compiler-cli/private/tooling.ts b/packages/compiler-cli/private/tooling.ts index 372e00198ca8..90e12bc0a76c 100644 --- a/packages/compiler-cli/private/tooling.ts +++ b/packages/compiler-cli/private/tooling.ts @@ -45,5 +45,5 @@ export function constructorParametersDownlevelTransform(program: ts.Program): const reflectionHost = new TypeScriptReflectionHost(typeChecker); return getDownlevelDecoratorsTransform( typeChecker, reflectionHost, [], /* isCore */ false, - /* enableClosureCompiler */ false, /* skipClassDecorators */ true); + /* enableClosureCompiler */ false); } diff --git a/packages/compiler-cli/src/transformers/downlevel_decorators_transform/downlevel_decorators_transform.ts b/packages/compiler-cli/src/transformers/downlevel_decorators_transform/downlevel_decorators_transform.ts index a01a4371dcf9..26f46e18140f 100644 --- a/packages/compiler-cli/src/transformers/downlevel_decorators_transform/downlevel_decorators_transform.ts +++ b/packages/compiler-cli/src/transformers/downlevel_decorators_transform/downlevel_decorators_transform.ts @@ -242,23 +242,20 @@ interface ParameterDecorationInfo { } /** - * Gets a transformer for downleveling Angular decorators. + * Gets a transformer for downleveling Angular constructor parameter and property decorators. + * + * Note that Angular class decorators are never processed as those rely on side effects that + * would otherwise no longer be executed. i.e. the creation of a component definition. + * * @param typeChecker Reference to the program's type checker. * @param host Reflection host that is used for determining decorators. * @param diagnostics List which will be populated with diagnostics if any. * @param isCore Whether the current TypeScript program is for the `@angular/core` package. * @param isClosureCompilerEnabled Whether closure annotations need to be added where needed. - * @param skipClassDecorators Whether class decorators should be skipped from downleveling. - * This is useful for JIT mode where class decorators should be preserved as they could rely - * on immediate execution. e.g. downleveling `@Injectable` means that the injectable factory - * is not created, and injecting the token will not work. If this decorator would not be - * downleveled, the `Injectable` decorator will execute immediately on file load, and - * Angular will generate the corresponding injectable factory. */ export function getDownlevelDecoratorsTransform( typeChecker: ts.TypeChecker, host: ReflectionHost, diagnostics: ts.Diagnostic[], - isCore: boolean, isClosureCompilerEnabled: boolean, - skipClassDecorators: boolean): ts.TransformerFactory { + isCore: boolean, isClosureCompilerEnabled: boolean): ts.TransformerFactory { function addJSDocTypeAnnotation(node: ts.Node, jsdocType: string): void { if (!isClosureCompilerEnabled) { return; @@ -275,26 +272,6 @@ export function getDownlevelDecoratorsTransform( ]); } - /** - * Takes a list of decorator metadata object ASTs and produces an AST for a - * static class property of an array of those metadata objects. - */ - function createDecoratorClassProperty(decoratorList: ts.ObjectLiteralExpression[]) { - const modifier = ts.factory.createToken(ts.SyntaxKind.StaticKeyword); - const initializer = ts.factory.createArrayLiteralExpression(decoratorList, true); - // NB: the .decorators property does not get a @nocollapse property. There - // is no good reason why - it means .decorators is not runtime accessible - // if you compile with collapse properties, whereas propDecorators is, - // which doesn't follow any stringent logic. However this has been the - // case previously, and adding it back in leads to substantial code size - // increases as Closure fails to tree shake these props - // without @nocollapse. - const prop = ts.factory.createPropertyDeclaration( - [modifier], 'decorators', undefined, undefined, initializer); - addJSDocTypeAnnotation(prop, DECORATOR_INVOCATION_JSDOC_TYPE); - return prop; - } - /** * createPropDecoratorsClassProperty creates a static 'propDecorators' * property containing type information for every property that has a @@ -523,36 +500,15 @@ export function getDownlevelDecoratorsTransform( newMembers.push(ts.visitEachChild(member, decoratorDownlevelVisitor, context)); } - // The `ReflectionHost.getDecoratorsOfDeclaration()` method will not return certain kinds of - // decorators that will never be Angular decorators. So we cannot rely on it to capture all - // the decorators that should be kept. Instead we start off with a set of the raw decorators - // on the class, and only remove the ones that have been identified for downleveling. - const decoratorsToKeep = new Set(ts.getDecorators(classDecl)); + // Note: The `ReflectionHost.getDecoratorsOfDeclaration()` method will not + // return all decorators, but only ones that could be possible Angular decorators. const possibleAngularDecorators = host.getDecoratorsOfDeclaration(classDecl) || []; - let hasAngularDecorator = false; - const decoratorsToLower = []; - for (const decorator of possibleAngularDecorators) { - // We only deal with concrete nodes in TypeScript sources, so we don't - // need to handle synthetically created decorators. - const decoratorNode = decorator.node! as ts.Decorator; - const isNgDecorator = isAngularDecorator(decorator, isCore); - - // Keep track if we come across an Angular class decorator. This is used - // to determine whether constructor parameters should be captured or not. - if (isNgDecorator) { - hasAngularDecorator = true; - } + // Keep track if we come across an Angular class decorator. This is used + // to determine whether constructor parameters should be captured or not. + const hasAngularDecorator = + possibleAngularDecorators.some(d => isAngularDecorator(d, isCore)); - if (isNgDecorator && !skipClassDecorators) { - decoratorsToLower.push(extractMetadataFromSingleDecorator(decoratorNode, diagnostics)); - decoratorsToKeep.delete(decoratorNode); - } - } - - if (decoratorsToLower.length) { - newMembers.push(createDecoratorClassProperty(decoratorsToLower)); - } if (classParameters) { if (hasAngularDecorator || classParameters.some(p => !!p.decorators.length)) { // Capture constructor parameters if the class has Angular decorator applied, @@ -568,16 +524,10 @@ export function getDownlevelDecoratorsTransform( const members = ts.setTextRange( ts.factory.createNodeArray(newMembers, classDecl.members.hasTrailingComma), classDecl.members); - const classModifiers = ts.getModifiers(classDecl); - let modifiers: ts.ModifierLike[]|undefined; - - if (decoratorsToKeep.size || classModifiers?.length) { - modifiers = [...decoratorsToKeep, ...(classModifiers || [])]; - } return ts.factory.updateClassDeclaration( - classDecl, modifiers, classDecl.name, classDecl.typeParameters, classDecl.heritageClauses, - members); + classDecl, classDecl.modifiers, classDecl.name, classDecl.typeParameters, + classDecl.heritageClauses, members); } /** diff --git a/packages/compiler-cli/test/downlevel_decorators_transform_spec.ts b/packages/compiler-cli/test/downlevel_decorators_transform_spec.ts index 50385715ee5f..b012bc06ff5f 100644 --- a/packages/compiler-cli/test/downlevel_decorators_transform_spec.ts +++ b/packages/compiler-cli/test/downlevel_decorators_transform_spec.ts @@ -22,7 +22,6 @@ describe('downlevel decorator transform', () => { let context: MockAotContext; let diagnostics: ts.Diagnostic[]; let isClosureEnabled: boolean; - let skipClassDecorators: boolean; beforeEach(() => { diagnostics = []; @@ -34,7 +33,6 @@ describe('downlevel decorator transform', () => { }); host = new MockCompilerHost(context); isClosureEnabled = false; - skipClassDecorators = false; }); function transform( @@ -61,7 +59,7 @@ describe('downlevel decorator transform', () => { ...preTransformers, getDownlevelDecoratorsTransform( program.getTypeChecker(), reflectionHost, diagnostics, - /* isCore */ false, isClosureEnabled, skipClassDecorators) + /* isCore */ false, isClosureEnabled) ] }; let output: string|null = null; @@ -97,13 +95,13 @@ describe('downlevel decorator transform', () => { expect(diagnostics.length).toBe(0); expect(output).toContain(dedent` - MyService.decorators = [ - { type: core_1.Injectable } - ]; MyService.ctorParameters = () => [ { type: ClassInject } - ];`); - expect(output).not.toContain('tslib'); + ]; + MyService = tslib_1.__decorate([ + (0, core_1.Injectable)() + ], MyService); + `); }); it('should downlevel decorators for @Directive decorated class', () => { @@ -120,13 +118,13 @@ describe('downlevel decorator transform', () => { expect(diagnostics.length).toBe(0); expect(output).toContain(dedent` - MyDir.decorators = [ - { type: core_1.Directive } - ]; MyDir.ctorParameters = () => [ { type: ClassInject } - ];`); - expect(output).not.toContain('tslib'); + ]; + MyDir = tslib_1.__decorate([ + (0, core_1.Directive)() + ], MyDir); + `); }); it('should downlevel decorators for @Component decorated class', () => { @@ -143,13 +141,12 @@ describe('downlevel decorator transform', () => { expect(diagnostics.length).toBe(0); expect(output).toContain(dedent` - MyComp.decorators = [ - { type: core_1.Component, args: [{ template: 'hello' },] } - ]; MyComp.ctorParameters = () => [ { type: ClassInject } - ];`); - expect(output).not.toContain('tslib'); + ]; + MyComp = tslib_1.__decorate([ + (0, core_1.Component)({ template: 'hello' }) + ], MyComp);`); }); it('should downlevel decorators for @Pipe decorated class', () => { @@ -166,13 +163,13 @@ describe('downlevel decorator transform', () => { expect(diagnostics.length).toBe(0); expect(output).toContain(dedent` - MyPipe.decorators = [ - { type: core_1.Pipe, args: [{ selector: 'hello' },] } - ]; MyPipe.ctorParameters = () => [ { type: ClassInject } - ];`); - expect(output).not.toContain('tslib'); + ]; + MyPipe = tslib_1.__decorate([ + (0, core_1.Pipe)({ selector: 'hello' }) + ], MyPipe); + `); }); it('should not downlevel non-Angular class decorators', () => { @@ -243,10 +240,9 @@ describe('downlevel decorator transform', () => { // would be processed twice, where the downleveled class is revisited accidentally and // caused invalid generation of the `ctorParameters` static class member. it('should not duplicate constructor parameters for classes part of constructor body', () => { - // The bug with duplicated/invalid generation only surfaces when the actual class + // Note: the bug with duplicated/invalid generation only surfaces when the actual class // decorators are preserved and emitted by TypeScript itself. This setting is also // disabled within the CLI. - skipClassDecorators = true; const {output} = transform(` import {Injectable} from '@angular/core'; @@ -324,12 +320,13 @@ describe('downlevel decorator transform', () => { expect(diagnostics.length).toBe(0); expect(output).toContain('const other_file_1 = require("./other-file");'); expect(output).toContain(dedent` - MyDir.decorators = [ - { type: core_1.Directive } - ]; MyDir.ctorParameters = () => [ { type: other_file_1.MyOtherClass } ]; + MyDir = tslib_1.__decorate([ + (0, core_1.Directive)(), + tslib_1.__metadata("design:paramtypes", [other_file_1.MyOtherClass]) + ], MyDir); `); }); @@ -350,12 +347,12 @@ describe('downlevel decorator transform', () => { expect(diagnostics.length).toBe(0); expect(output).toContain('const other_file_1 = require("./other-file");'); expect(output).toContain(dedent` - MyDir.decorators = [ - { type: core_1.Directive } - ]; MyDir.ctorParameters = () => [ { type: other_file_1.MyOtherClass } ]; + MyDir = tslib_1.__decorate([ + (0, core_1.Directive)() + ], MyDir); `); expect(dtsOutput).toContain('import'); }); @@ -375,12 +372,12 @@ describe('downlevel decorator transform', () => { expect(diagnostics.length).toBe(0); expect(output).toContain('const externalFile = require("./other-file");'); expect(output).toContain(dedent` - MyDir.decorators = [ - { type: core_1.Directive } - ]; MyDir.ctorParameters = () => [ { type: externalFile.MyOtherClass } ]; + MyDir = tslib_1.__decorate([ + (0, core_1.Directive)() + ], MyDir); `); }); @@ -401,12 +398,12 @@ describe('downlevel decorator transform', () => { expect(diagnostics.length).toBe(0); expect(output).toContain('var other;'); expect(output).toContain(dedent` - MyDir.decorators = [ - { type: core_1.Directive } - ]; MyDir.ctorParameters = () => [ { type: other.OtherClass } ]; + MyDir = tslib_1.__decorate([ + (0, core_1.Directive)() + ], MyDir); `); }); @@ -422,12 +419,12 @@ describe('downlevel decorator transform', () => { expect(diagnostics.length).toBe(0); expect(output).toContain(dedent` - MyDir.decorators = [ - { type: core_1.Directive } - ]; MyDir.ctorParameters = () => [ { type: Document, decorators: [{ type: core_1.Inject, args: [core_1.DOCUMENT,] }] } ]; + MyDir = tslib_1.__decorate([ + (0, core_1.Directive)() + ], MyDir); `); }); @@ -443,12 +440,12 @@ describe('downlevel decorator transform', () => { expect(diagnostics.length).toBe(0); expect(output).toContain(dedent` - MyDir.decorators = [ - { type: core_1.Directive } - ]; MyDir.ctorParameters = () => [ { type: core_1.NgZone, decorators: [{ type: core_1.Optional }] } ]; + MyDir = tslib_1.__decorate([ + (0, core_1.Directive)() + ], MyDir); `); }); @@ -467,10 +464,9 @@ describe('downlevel decorator transform', () => { expect(diagnostics.length).toBe(0); expect(output).toContain(dedent` - /** @type {!Array<{type: !Function, args: (undefined|!Array)}>} */ - MyDir.decorators = [ - { type: core_1.Directive } - ]; + MyDir = tslib_1.__decorate([ + (0, core_1.Directive)() + ], MyDir); `); expect(output).toContain(dedent` /** @@ -484,7 +480,6 @@ describe('downlevel decorator transform', () => { { type: ClassInject } ]; `); - expect(output).not.toContain('tslib'); }); it('should not retain unused type imports due to decorator downleveling with ' + @@ -496,19 +491,18 @@ describe('downlevel decorator transform', () => { `); const {output} = transform( ` - import {Directive} from '@angular/core'; + import {Directive, Inject} from '@angular/core'; import {ErrorHandler, ClassInject} from './external'; - @Directive() export class MyDir { private _errorHandler: ErrorHandler; - constructor(v: ClassInject) {} + constructor(@Inject(ClassInject) i: ClassInject) {} } `, {module: ts.ModuleKind.ES2015, emitDecoratorMetadata: true}); expect(diagnostics.length).toBe(0); - expect(output).not.toContain('tslib'); + expect(output).not.toContain('Directive'); expect(output).not.toContain('ErrorHandler'); }); @@ -521,19 +515,18 @@ describe('downlevel decorator transform', () => { `); const {output} = transform( ` - import {Directive} from '@angular/core'; + import {Directive, Inject} from '@angular/core'; import {ErrorHandler, ClassInject} from './external'; - @Directive() export class MyDir { private _errorHandler: ErrorHandler; - constructor(v: ClassInject) {} + constructor(@Inject(ClassInject) i: ClassInject) {} } `, {module: ts.ModuleKind.ES2015, emitDecoratorMetadata: false}); expect(diagnostics.length).toBe(0); - expect(output).not.toContain('tslib'); + expect(output).not.toContain('Directive'); expect(output).not.toContain('ErrorHandler'); }); @@ -558,15 +551,14 @@ describe('downlevel decorator transform', () => { {emitDecoratorMetadata: false}); expect(diagnostics.length).toBe(0); - expect(output).not.toContain('tslib'); expect(output).toContain(`external_1 = require("./external");`); expect(output).toContain(dedent` - MyDir.decorators = [ - { type: core_1.Directive } - ]; MyDir.ctorParameters = () => [ { type: external_1.Dep } ]; + MyDir = tslib_1.__decorate([ + (0, core_1.Directive)() + ], MyDir); `); }); @@ -582,12 +574,15 @@ describe('downlevel decorator transform', () => { expect(diagnostics.length).toBe(0); expect(output).toContain(dedent` - MyDir.decorators = [ - { type: core_1.Directive } - ]; + let MyDir = MyDir_1 = class MyDir { + constructor(parentDir) { } + }; MyDir.ctorParameters = () => [ - { type: MyDir, decorators: [{ type: core_1.Optional }, { type: core_1.SkipSelf }, { type: core_1.Inject, args: [MyDir,] }] } + { type: MyDir, decorators: [{ type: core_1.Optional }, { type: core_1.SkipSelf }, { type: core_1.Inject, args: [MyDir_1,] }] } ]; + MyDir = MyDir_1 = tslib_1.__decorate([ + (0, core_1.Directive)() + ], MyDir); `); }); @@ -633,15 +628,15 @@ describe('downlevel decorator transform', () => { expect(diagnostics.length).toBe(0); expect(output).not.toContain('external'); expect(output).toContain(dedent` - MyDir.decorators = [ - { type: core_1.Directive } - ]; MyDir.ctorParameters = () => [ { type: undefined, decorators: [{ type: core_1.Inject, args: ['$state',] }] }, { type: undefined, decorators: [{ type: core_1.Inject, args: ['$overlay',] }] }, { type: undefined, decorators: [{ type: core_1.Inject, args: ['$default',] }] }, { type: undefined, decorators: [{ type: core_1.Inject, args: ['$keyCodes',] }] } ]; + MyDir = tslib_1.__decorate([ + (0, core_1.Directive)() + ], MyDir); `); }); @@ -693,13 +688,13 @@ describe('downlevel decorator transform', () => { expect(diagnostics.length).toBe(0); expect(output).toContain(dedent` - MyComp.decorators = [ - { type: core_1.Component, args: [{ template: 'hello' },] } - ]; MyComp.ctorParameters = () => [ { type: Values } - ];`); - expect(output).not.toContain('tslib'); + ]; + MyComp = tslib_1.__decorate([ + (0, core_1.Component)({ template: 'hello' }) + ], MyComp); + `); }); it('should allow for type-only references to be removed with `emitDecoratorMetadata` from custom decorators', @@ -729,97 +724,6 @@ describe('downlevel decorator transform', () => { expect(output).toContain('metadata("design:returntype", Object)'); }); - describe('class decorators skipped', () => { - beforeEach(() => skipClassDecorators = true); - - it('should not downlevel Angular class decorators', () => { - const {output} = transform(` - import {Injectable} from '@angular/core'; - - @Injectable() - export class MyService {} - `); - - expect(diagnostics.length).toBe(0); - expect(output).not.toContain('MyService.decorators'); - expect(output).toContain(dedent` - MyService = tslib_1.__decorate([ - (0, core_1.Injectable)() - ], MyService); - `); - }); - - it('should downlevel constructor parameters', () => { - const {output} = transform(` - import {Injectable} from '@angular/core'; - - @Injectable() - export class InjectClass {} - - @Injectable() - export class MyService { - constructor(dep: InjectClass) {} - } - `); - - expect(diagnostics.length).toBe(0); - expect(output).not.toContain('MyService.decorators'); - expect(output).toContain('MyService.ctorParameters'); - expect(output).toContain(dedent` - MyService.ctorParameters = () => [ - { type: InjectClass } - ]; - MyService = tslib_1.__decorate([ - (0, core_1.Injectable)() - ], MyService); - `); - }); - - it('should downlevel constructor parameter decorators', () => { - const {output} = transform(` - import {Injectable, Inject} from '@angular/core'; - - @Injectable() - export class InjectClass {} - - @Injectable() - export class MyService { - constructor(@Inject('test') dep: InjectClass) {} - } - `); - - expect(diagnostics.length).toBe(0); - expect(output).not.toContain('MyService.decorators'); - expect(output).toContain('MyService.ctorParameters'); - expect(output).toContain(dedent` - MyService.ctorParameters = () => [ - { type: InjectClass, decorators: [{ type: core_1.Inject, args: ['test',] }] } - ]; - MyService = tslib_1.__decorate([ - (0, core_1.Injectable)() - ], MyService); - `); - }); - - it('should downlevel class member Angular decorators', () => { - const {output} = transform(` - import {Injectable, Input} from '@angular/core'; - - export class MyService { - @Input() disabled: boolean; - } - `); - - expect(diagnostics.length).toBe(0); - expect(output).not.toContain('tslib'); - expect(output).toContain(dedent` - MyService.propDecorators = { - disabled: [{ type: core_1.Input }] - }; - `); - }); - }); - describe('transforming multiple files', () => { it('should work correctly for multiple files that import distinct declarations', () => { context.writeFile('foo_service.d.ts', ` @@ -915,7 +819,7 @@ describe('downlevel decorator transform', () => { const transformers: ts.CustomTransformers = { before: [getDownlevelDecoratorsTransform( program.getTypeChecker(), reflectionHost, diagnostics, - /* isCore */ false, isClosureEnabled, skipClassDecorators)] + /* isCore */ false, isClosureEnabled)] }; return {program, transformers}; } diff --git a/tools/legacy-saucelabs/downlevel_decorator_transform.ts b/tools/legacy-saucelabs/downlevel_decorator_transform.ts index e675dff608bf..b5f654ffc051 100644 --- a/tools/legacy-saucelabs/downlevel_decorator_transform.ts +++ b/tools/legacy-saucelabs/downlevel_decorator_transform.ts @@ -23,5 +23,5 @@ export function legacyCompilationDownlevelDecoratorTransform(program: ts.Program // Note: `isCore` is set to `true` since we also process the core package. return getDownlevelDecoratorsTransform( typeChecker, reflectionHost, [], /* isCore */ true, - /* enableClosureCompiler */ false, /* skipClassDecorators */ true); + /* enableClosureCompiler */ false); } diff --git a/yarn.lock b/yarn.lock index ea6f0f83d082..7852fc97b096 100644 --- a/yarn.lock +++ b/yarn.lock @@ -11957,13 +11957,6 @@ madge@^6.0.0: typescript "^3.9.5" walkdir "^0.4.1" -magic-string@0.27.0, magic-string@^0.27.0: - version "0.27.0" - resolved "https://registry.yarnpkg.com/magic-string/-/magic-string-0.27.0.tgz#e4a3413b4bab6d98d2becffd48b4a257effdbbf3" - integrity sha512-8UnnX2PeRAPZuN12svgR9j7M1uWMovg/CEnIwIG0LFkXSJJe4PdfUGiTGl8V9bsBHFUtfVINcSyYxd7q+kx9fA== - dependencies: - "@jridgewell/sourcemap-codec" "^1.4.13" - magic-string@0.30.0: version "0.30.0" resolved "https://registry.yarnpkg.com/magic-string/-/magic-string-0.30.0.tgz#fd58a4748c5c4547338a424e90fa5dd17f4de529" @@ -11978,6 +11971,13 @@ magic-string@^0.25.7: dependencies: sourcemap-codec "^1.4.8" +magic-string@^0.27.0: + version "0.27.0" + resolved "https://registry.yarnpkg.com/magic-string/-/magic-string-0.27.0.tgz#e4a3413b4bab6d98d2becffd48b4a257effdbbf3" + integrity sha512-8UnnX2PeRAPZuN12svgR9j7M1uWMovg/CEnIwIG0LFkXSJJe4PdfUGiTGl8V9bsBHFUtfVINcSyYxd7q+kx9fA== + dependencies: + "@jridgewell/sourcemap-codec" "^1.4.13" + make-dir@^1.0.0: version "1.3.0" resolved "https://registry.yarnpkg.com/make-dir/-/make-dir-1.3.0.tgz#79c1033b80515bd6d24ec9933e860ca75ee27f0c" From 75d5bfda1ea5396fa5210e978af1217405e4a041 Mon Sep 17 00:00:00 2001 From: Jessica Janiuk Date: Wed, 8 Mar 2023 10:38:46 -0800 Subject: [PATCH 32/32] release: cut the v15.2.2 release --- CHANGELOG.md | 11 +++++++++++ package.json | 2 +- 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fdd7773a4f7c..bcbb5214da4a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,14 @@ + +# 15.2.2 (2023-03-08) +### migrations +| Commit | Type | Description | +| -- | -- | -- | +| [6207d6f1f0](https://github.com/angular/angular/commit/6207d6f1f0771ff3b74379367e65af665ef0e51c) | fix | add protractor support if protractor imports are detected ([#49274](https://github.com/angular/angular/pull/49274)) | +## Special Thanks +Alan Agius, Andrew Kushnir, Andrew Scott, Kristiyan Kostadinov, Matthieu Riegler, Paul Gschwendtner, Sai Kartheek Bommisetty and Vinit Neogi + + + # 15.2.1 (2023-03-01) ### common diff --git a/package.json b/package.json index 98da20c3687f..ad4864242957 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "angular-srcs", - "version": "15.2.1", + "version": "15.2.2", "private": true, "description": "Angular - a web framework for modern web apps", "homepage": "https://github.com/angular/angular",