From 130c84ef7c041a2d0aadf0bbad0da2f9a3fe19f7 Mon Sep 17 00:00:00 2001 From: Tim Deschryver <28659384+timdeschryver@users.noreply.github.com> Date: Mon, 20 Nov 2023 15:44:47 +0100 Subject: [PATCH 01/19] docs: update defer test snippet (#53056) PR Close #53056 --- adev/src/content/guide/defer.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/adev/src/content/guide/defer.md b/adev/src/content/guide/defer.md index 13e06b19bef65..be842d1ee7f2e 100644 --- a/adev/src/content/guide/defer.md +++ b/adev/src/content/guide/defer.md @@ -262,7 +262,7 @@ it('should render a defer block in different states', async () => { const componentFixture = TestBed.createComponent(ComponentA); // Retrieve the list of all defer block fixtures and get the first block. - const deferBlockFixture = async (componentFixture.getDeferBlocks())[0]; + const deferBlockFixture = (await componentFixture.getDeferBlocks())[0]; // Renders placeholder state by default. expect(componentFixture.nativeElement.innerHTML).toContain('Placeholder'); @@ -272,7 +272,7 @@ it('should render a defer block in different states', async () => { expect(componentFixture.nativeElement.innerHTML).toContain('Loading'); // Render final state and verify the output. - await deferBlockFixture.render(DeferBlockState.Completed); + await deferBlockFixture.render(DeferBlockState.Complete); expect(componentFixture.nativeElement.innerHTML).toContain('large works!'); }); ``` From 54823893c83f48ccf54a769ade2b4163f2e04fee Mon Sep 17 00:00:00 2001 From: Tim Deschryver <28659384+timdeschryver@users.noreply.github.com> Date: Mon, 20 Nov 2023 15:47:59 +0100 Subject: [PATCH 02/19] docs: update defer testing snippet (#53056) PR Close #53056 --- aio/content/guide/defer.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/aio/content/guide/defer.md b/aio/content/guide/defer.md index 3d2ae23cd7aaf..0fed612e7dd67 100644 --- a/aio/content/guide/defer.md +++ b/aio/content/guide/defer.md @@ -262,7 +262,7 @@ it('should render a defer block in different states', async () => { const componentFixture = TestBed.createComponent(ComponentA); // Retrieve the list of all defer block fixtures and get the first block. - const deferBlockFixture = async (componentFixture.getDeferBlocks())[0]; + const deferBlockFixture = (await componentFixture.getDeferBlocks())[0]; // Renders placeholder state by default. expect(componentFixture.nativeElement.innerHTML).toContain('Placeholder'); @@ -272,7 +272,7 @@ it('should render a defer block in different states', async () => { expect(componentFixture.nativeElement.innerHTML).toContain('Loading'); // Render final state and verify the output. - await deferBlockFixture.render(DeferBlockState.Completed); + await deferBlockFixture.render(DeferBlockState.Complete); expect(componentFixture.nativeElement.innerHTML).toContain('large works!'); }); ``` From ed0fbd4071339b1af22d82bac07d51c6c41790cd Mon Sep 17 00:00:00 2001 From: arturovt Date: Sat, 18 Nov 2023 23:06:14 +0200 Subject: [PATCH 03/19] fix(core): cleanup loading promise when no dependencies are defined (#53031) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This commit cleans up the `loadingPromise` when no `dependenciesFn` is defined, as it's already cleaned up after the resolution of `Promise.allSettled`. This occurs with `prefetch on` triggers, such as when `triggerResourceLoading` is called from `ɵɵdeferPrefetchOnImmediate`, where there are no dependencies to load. The `loadingPromise` should still be cleaned up because it typically involves the `ZoneAwarePromise`, which isn't properly garbage collected when referenced elsewhere (in this case, it would be referenced from the `tView` data). PR Close #53031 --- packages/core/src/defer/instructions.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/core/src/defer/instructions.ts b/packages/core/src/defer/instructions.ts index bb31ab813881f..c6759f3cfe8a1 100644 --- a/packages/core/src/defer/instructions.ts +++ b/packages/core/src/defer/instructions.ts @@ -655,10 +655,11 @@ export function triggerResourceLoading(tDetails: TDeferBlockDetails, lView: LVie } // The `dependenciesFn` might be `null` when all dependencies within - // a given defer block were eagerly references elsewhere in a file, + // a given defer block were eagerly referenced elsewhere in a file, // thus no dynamic `import()`s were produced. if (!dependenciesFn) { tDetails.loadingPromise = Promise.resolve().then(() => { + tDetails.loadingPromise = null; tDetails.loadingState = DeferDependenciesLoadingState.COMPLETE; }); return; From f84057d366c07d2f3f4ab76abe1a0ff55236b9de Mon Sep 17 00:00:00 2001 From: Nicolas Frizzarin Date: Sat, 18 Nov 2023 17:58:46 +0100 Subject: [PATCH 04/19] docs: example code afterNextRender phase (#53025) Currently in the angular.dev documentation, afterNextRender function take directly the phase as a second parameter which is not correct. According to the api of this hook, the second parameter is an object which contains the phase. This PR update the part of the documentation PR Close #53025 --- adev/src/content/guide/components/lifecycle.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/adev/src/content/guide/components/lifecycle.md b/adev/src/content/guide/components/lifecycle.md index 8a9ce66da311c..7bf271e702f54 100644 --- a/adev/src/content/guide/components/lifecycle.md +++ b/adev/src/content/guide/components/lifecycle.md @@ -257,12 +257,12 @@ export class UserProfile { // Use the `Write` phase to write to a geometric property. afterNextRender(() => { nativeElement.style.padding = computePadding(); - }, AfterRenderPhase.Write); + }, {phase: AfterRenderPhase.Write}); // Use the `Read` phase to read geometric properties after all writes have occured. afterNextRender(() => { this.elementHeight = nativeElement.getBoundingClientRect().height; - }, AfterRenderPhase.Read); + }, {phase: AfterRenderPhase.Read}); } } ``` From 5564d020cdcea8273b65cf69c45c3f935195af66 Mon Sep 17 00:00:00 2001 From: Jessica Janiuk Date: Fri, 17 Nov 2023 11:54:20 -0500 Subject: [PATCH 05/19] fix(migrations): Fixes control flow migration if then else case (#53006) With if then else use cases, we now properly account for the length of the original element's contents when tracking new offsets. fixes: #52927 PR Close #53006 --- .../ng-generate/control-flow-migration/ifs.ts | 4 +- .../control-flow-migration/util.ts | 11 ++-- .../test/control_flow_migration_spec.ts | 53 +++++++++++++++++++ 3 files changed, 63 insertions(+), 5 deletions(-) diff --git a/packages/core/schematics/ng-generate/control-flow-migration/ifs.ts b/packages/core/schematics/ng-generate/control-flow-migration/ifs.ts index faa917dc8d1ad..34527b4a9b546 100644 --- a/packages/core/schematics/ng-generate/control-flow-migration/ifs.ts +++ b/packages/core/schematics/ng-generate/control-flow-migration/ifs.ts @@ -175,7 +175,9 @@ function buildIfThenElseBlock( const updatedTmpl = tmplStart + ifThenElseBlock + tmplEnd; - const pre = originals.start.length - startBlock.length; + // We ignore the contents of the element on if then else. + // If there's anything there, we need to account for the length in the offset. + const pre = originals.start.length + originals.childLength - startBlock.length; const post = originals.end.length - postBlock.length; return {tmpl: updatedTmpl, offsets: {pre, post}}; diff --git a/packages/core/schematics/ng-generate/control-flow-migration/util.ts b/packages/core/schematics/ng-generate/control-flow-migration/util.ts index 35297ce780796..e925b8e0752b2 100644 --- a/packages/core/schematics/ng-generate/control-flow-migration/util.ts +++ b/packages/core/schematics/ng-generate/control-flow-migration/util.ts @@ -336,10 +336,12 @@ export function removeImports( * retrieves the original block of text in the template for length comparison during migration * processing */ -export function getOriginals( - etm: ElementToMigrate, tmpl: string, offset: number): {start: string, end: string} { +export function getOriginals(etm: ElementToMigrate, tmpl: string, offset: number): + {start: string, end: string, childLength: number} { // original opening block if (etm.el.children.length > 0) { + const childStart = etm.el.children[0].sourceSpan.start.offset - offset; + const childEnd = etm.el.children[etm.el.children.length - 1].sourceSpan.end.offset - offset; const start = tmpl.slice( etm.el.sourceSpan.start.offset - offset, etm.el.children[0].sourceSpan.start.offset - offset); @@ -347,13 +349,14 @@ export function getOriginals( const end = tmpl.slice( etm.el.children[etm.el.children.length - 1].sourceSpan.end.offset - offset, etm.el.sourceSpan.end.offset - offset); - return {start, end}; + const childLength = childEnd - childStart; + return {start, end, childLength}; } // self closing or no children const start = tmpl.slice(etm.el.sourceSpan.start.offset - offset, etm.el.sourceSpan.end.offset - offset); // original closing block - return {start, end: ''}; + return {start, end: '', childLength: 0}; } function isI18nTemplate(etm: ElementToMigrate, i18nAttr: Attribute|undefined): boolean { diff --git a/packages/core/schematics/test/control_flow_migration_spec.ts b/packages/core/schematics/test/control_flow_migration_spec.ts index 6a67ad659141c..00107f3b9aca8 100644 --- a/packages/core/schematics/test/control_flow_migration_spec.ts +++ b/packages/core/schematics/test/control_flow_migration_spec.ts @@ -828,6 +828,59 @@ describe('control flow migration', () => { ].join('\n')); }); + it('should migrate a complex if then else case on ng-containers', async () => { + writeFile('/comp.ts', ` + import {Component} from '@angular/core'; + import {NgIf} from '@angular/common'; + + @Component({ + templateUrl: './comp.html' + }) + class Comp { + show = false; + } + `); + + writeFile('/comp.html', [ + ``, + ` `, + ` `, + ``, + ` `, + `
`, + ` `, + ``, + ``, + ` Empty`, + ``, + ``, + ``, + `

Loading

`, + ``, + ].join('\n')); + + await runMigration(); + const content = tree.readContent('/comp.html'); + + expect(content).toBe([ + ``, + ` @if (loading) {`, + `

Loading

`, + ` } @else {`, + ` @if (selected) {`, + `
`, + ` } @else {`, + ` Empty`, + ` }`, + ` }`, + ``, + ].join('\n')); + }); + it('should migrate but not remove ng-templates when referenced elsewhere', async () => { writeFile('/comp.ts', ` import {Component} from '@angular/core'; From 28f6cbf9c91f957b4926fe34610387e1f1919d4f Mon Sep 17 00:00:00 2001 From: Jessica Janiuk Date: Fri, 17 Nov 2023 13:41:30 -0500 Subject: [PATCH 06/19] fix(migrations): fixes migrations of nested switches in control flow (#53010) This separates out the NgSwitch migration pass from the NgSwitchCase / Default pass, which makes nested switch migrations work. fixes: #53009 PR Close #53010 --- .../control-flow-migration/cases.ts | 122 ++++++++++++++++++ .../control-flow-migration/migration.ts | 5 +- .../control-flow-migration/switches.ts | 68 ---------- .../test/control_flow_migration_spec.ts | 52 ++++++++ 4 files changed, 178 insertions(+), 69 deletions(-) create mode 100644 packages/core/schematics/ng-generate/control-flow-migration/cases.ts diff --git a/packages/core/schematics/ng-generate/control-flow-migration/cases.ts b/packages/core/schematics/ng-generate/control-flow-migration/cases.ts new file mode 100644 index 0000000000000..dc50096034561 --- /dev/null +++ b/packages/core/schematics/ng-generate/control-flow-migration/cases.ts @@ -0,0 +1,122 @@ +/** + * @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 {visitAll} from '@angular/compiler'; + +import {ElementCollector, ElementToMigrate, MigrateError, Result} from './types'; +import {calculateNesting, getMainBlock, getOriginals, hasLineBreaks, parseTemplate, reduceNestingOffset} from './util'; + +export const boundcase = '[ngSwitchCase]'; +export const switchcase = '*ngSwitchCase'; +export const nakedcase = 'ngSwitchCase'; +export const switchdefault = '*ngSwitchDefault'; +export const nakeddefault = 'ngSwitchDefault'; + +const cases = [ + boundcase, + switchcase, + nakedcase, + switchdefault, + nakeddefault, +]; + +/** + * Replaces structural directive ngSwitch instances with new switch. + * Returns null if the migration failed (e.g. there was a syntax error). + */ +export function migrateCase(template: string): {migrated: string, errors: MigrateError[]} { + let errors: MigrateError[] = []; + let parsed = parseTemplate(template); + if (parsed === null) { + return {migrated: template, errors}; + } + + let result = template; + const visitor = new ElementCollector(cases); + visitAll(visitor, parsed.rootNodes); + calculateNesting(visitor, hasLineBreaks(template)); + + // this tracks the character shift from different lengths of blocks from + // the prior directives so as to adjust for nested block replacement during + // migration. Each block calculates length differences and passes that offset + // to the next migrating block to adjust character offsets properly. + let offset = 0; + let nestLevel = -1; + let postOffsets: number[] = []; + for (const el of visitor.elements) { + let migrateResult: Result = {tmpl: result, offsets: {pre: 0, post: 0}}; + // applies the post offsets after closing + offset = reduceNestingOffset(el, nestLevel, offset, postOffsets); + + if (el.attr.name === switchcase || el.attr.name === nakedcase || el.attr.name === boundcase) { + try { + migrateResult = migrateNgSwitchCase(el, result, offset); + } catch (error: unknown) { + errors.push({type: switchcase, error}); + } + } else if (el.attr.name === switchdefault || el.attr.name === nakeddefault) { + try { + migrateResult = migrateNgSwitchDefault(el, result, offset); + } catch (error: unknown) { + errors.push({type: switchdefault, error}); + } + } + + result = migrateResult.tmpl; + offset += migrateResult.offsets.pre; + postOffsets.push(migrateResult.offsets.post); + nestLevel = el.nestCount; + } + + return {migrated: result, errors}; +} + +function migrateNgSwitchCase(etm: ElementToMigrate, tmpl: string, offset: number): Result { + // includes the mandatory semicolon before as + const lbString = etm.hasLineBreaks ? '\n' : ''; + const leadingSpace = etm.hasLineBreaks ? '' : ' '; + const condition = etm.attr.value; + + const originals = getOriginals(etm, tmpl, offset); + + const {start, middle, end} = getMainBlock(etm, tmpl, offset); + const startBlock = `${leadingSpace}@case (${condition}) {${leadingSpace}${lbString}${start}`; + const endBlock = `${end}${lbString}${leadingSpace}}`; + + const defaultBlock = startBlock + middle + endBlock; + const updatedTmpl = tmpl.slice(0, etm.start(offset)) + defaultBlock + tmpl.slice(etm.end(offset)); + + // this should be the difference between the starting element up to the start of the closing + // element and the mainblock sans } + const pre = originals.start.length - startBlock.length; + const post = originals.end.length - endBlock.length; + + return {tmpl: updatedTmpl, offsets: {pre, post}}; +} + +function migrateNgSwitchDefault(etm: ElementToMigrate, tmpl: string, offset: number): Result { + // includes the mandatory semicolon before as + const lbString = etm.hasLineBreaks ? '\n' : ''; + const leadingSpace = etm.hasLineBreaks ? '' : ' '; + + const originals = getOriginals(etm, tmpl, offset); + + const {start, middle, end} = getMainBlock(etm, tmpl, offset); + const startBlock = `${leadingSpace}@default {${leadingSpace}${lbString}${start}`; + const endBlock = `${end}${lbString}${leadingSpace}}`; + + const defaultBlock = startBlock + middle + endBlock; + const updatedTmpl = tmpl.slice(0, etm.start(offset)) + defaultBlock + tmpl.slice(etm.end(offset)); + + // this should be the difference between the starting element up to the start of the closing + // element and the mainblock sans } + const pre = originals.start.length - startBlock.length; + const post = originals.end.length - endBlock.length; + + return {tmpl: updatedTmpl, offsets: {pre, post}}; +} diff --git a/packages/core/schematics/ng-generate/control-flow-migration/migration.ts b/packages/core/schematics/ng-generate/control-flow-migration/migration.ts index cd4653f9dc47c..09d9f81f4d155 100644 --- a/packages/core/schematics/ng-generate/control-flow-migration/migration.ts +++ b/packages/core/schematics/ng-generate/control-flow-migration/migration.ts @@ -8,6 +8,7 @@ import ts from 'typescript'; +import {migrateCase} from './cases'; import {migrateFor} from './fors'; import {migrateIf} from './ifs'; import {migrateSwitch} from './switches'; @@ -26,7 +27,8 @@ export function migrateTemplate( const ifResult = migrateIf(template); const forResult = migrateFor(ifResult.migrated); const switchResult = migrateSwitch(forResult.migrated); - migrated = processNgTemplates(switchResult.migrated); + const caseResult = migrateCase(switchResult.migrated); + migrated = processNgTemplates(caseResult.migrated); if (format) { migrated = formatTemplate(migrated); } @@ -36,6 +38,7 @@ export function migrateTemplate( ...ifResult.errors, ...forResult.errors, ...switchResult.errors, + ...caseResult.errors, ]; } else { migrated = removeImports(template, node, file.removeCommonModule); diff --git a/packages/core/schematics/ng-generate/control-flow-migration/switches.ts b/packages/core/schematics/ng-generate/control-flow-migration/switches.ts index 3d8745b5190bb..6656f501bd8a6 100644 --- a/packages/core/schematics/ng-generate/control-flow-migration/switches.ts +++ b/packages/core/schematics/ng-generate/control-flow-migration/switches.ts @@ -12,19 +12,9 @@ import {ElementCollector, ElementToMigrate, MigrateError, Result} from './types' import {calculateNesting, getMainBlock, getOriginals, hasLineBreaks, parseTemplate, reduceNestingOffset} from './util'; export const ngswitch = '[ngSwitch]'; -export const boundcase = '[ngSwitchCase]'; -export const switchcase = '*ngSwitchCase'; -export const nakedcase = 'ngSwitchCase'; -export const switchdefault = '*ngSwitchDefault'; -export const nakeddefault = 'ngSwitchDefault'; const switches = [ ngswitch, - boundcase, - switchcase, - nakedcase, - switchdefault, - nakeddefault, ]; /** @@ -61,19 +51,6 @@ export function migrateSwitch(template: string): {migrated: string, errors: Migr } catch (error: unknown) { errors.push({type: ngswitch, error}); } - } else if ( - el.attr.name === switchcase || el.attr.name === nakedcase || el.attr.name === boundcase) { - try { - migrateResult = migrateNgSwitchCase(el, result, offset); - } catch (error: unknown) { - errors.push({type: ngswitch, error}); - } - } else if (el.attr.name === switchdefault || el.attr.name === nakeddefault) { - try { - migrateResult = migrateNgSwitchDefault(el, result, offset); - } catch (error: unknown) { - errors.push({type: ngswitch, error}); - } } result = migrateResult.tmpl; @@ -105,48 +82,3 @@ function migrateNgSwitch(etm: ElementToMigrate, tmpl: string, offset: number): R return {tmpl: updatedTmpl, offsets: {pre, post}}; } - -function migrateNgSwitchCase(etm: ElementToMigrate, tmpl: string, offset: number): Result { - // includes the mandatory semicolon before as - const lbString = etm.hasLineBreaks ? '\n' : ''; - const leadingSpace = etm.hasLineBreaks ? '' : ' '; - const condition = etm.attr.value; - - const originals = getOriginals(etm, tmpl, offset); - - const {start, middle, end} = getMainBlock(etm, tmpl, offset); - const startBlock = `${leadingSpace}@case (${condition}) {${leadingSpace}${lbString}${start}`; - const endBlock = `${end}${lbString}${leadingSpace}}`; - - const defaultBlock = startBlock + middle + endBlock; - const updatedTmpl = tmpl.slice(0, etm.start(offset)) + defaultBlock + tmpl.slice(etm.end(offset)); - - // this should be the difference between the starting element up to the start of the closing - // element and the mainblock sans } - const pre = originals.start.length - startBlock.length; - const post = originals.end.length - endBlock.length; - - return {tmpl: updatedTmpl, offsets: {pre, post}}; -} - -function migrateNgSwitchDefault(etm: ElementToMigrate, tmpl: string, offset: number): Result { - // includes the mandatory semicolon before as - const lbString = etm.hasLineBreaks ? '\n' : ''; - const leadingSpace = etm.hasLineBreaks ? '' : ' '; - - const originals = getOriginals(etm, tmpl, offset); - - const {start, middle, end} = getMainBlock(etm, tmpl, offset); - const startBlock = `${leadingSpace}@default {${leadingSpace}${lbString}${start}`; - const endBlock = `${end}${lbString}${leadingSpace}}`; - - const defaultBlock = startBlock + middle + endBlock; - const updatedTmpl = tmpl.slice(0, etm.start(offset)) + defaultBlock + tmpl.slice(etm.end(offset)); - - // this should be the difference between the starting element up to the start of the closing - // element and the mainblock sans } - const pre = originals.start.length - startBlock.length; - const post = originals.end.length - endBlock.length; - - return {tmpl: updatedTmpl, offsets: {pre, post}}; -} diff --git a/packages/core/schematics/test/control_flow_migration_spec.ts b/packages/core/schematics/test/control_flow_migration_spec.ts index 00107f3b9aca8..5452e32edae20 100644 --- a/packages/core/schematics/test/control_flow_migration_spec.ts +++ b/packages/core/schematics/test/control_flow_migration_spec.ts @@ -2125,6 +2125,58 @@ describe('control flow migration', () => { expect(content).toContain( 'template: `
@switch (testOpts) { @case (1) {

Option 1

} @case (2) {

Option 2

}}
`'); }); + + it('should migrate nested switches', async () => { + writeFile('/comp.ts', ` + import {Component} from '@angular/core'; + import {NgIf} from '@angular/common'; + + @Component({ + imports: [NgFor, NgIf], + templateUrl: './comp.html' + }) + class Comp { + show = false; + nest = true; + again = true; + more = true; + }`); + + writeFile('/comp.html', [ + `
`, + `
`, + ` PNG`, + ` default`, + `
`, + ` default`, + `
`, + ].join('\n')); + + await runMigration(); + const content = tree.readContent('/comp.html'); + + expect(content).toBe([ + `
`, + ` @switch (thing) {`, + ` @case ('item') {`, + `
`, + ` @switch (anotherThing) {`, + ` @case ('png') {`, + ` PNG`, + ` }`, + ` @default {`, + ` default`, + ` }`, + ` }`, + `
`, + ` }`, + ` @default {`, + ` default`, + ` }`, + ` }`, + `
`, + ].join('\n')); + }); }); describe('nested structures', () => { From 7affa5775427e92ef6e949c879765b7c8aa172da Mon Sep 17 00:00:00 2001 From: arturovt Date: Fri, 17 Nov 2023 12:23:12 +0200 Subject: [PATCH 07/19] fix(common): scan images once page is loaded (#52991) This commit updates the implementation of the `ImagePerformanceWarning` and runs the image scan even if the page has already been loaded. The `window.load` event would never fire if the page has already been loaded; that's why we're checking for the document's ready state. PR Close #52991 --- packages/core/src/image_performance_warning.ts | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/packages/core/src/image_performance_warning.ts b/packages/core/src/image_performance_warning.ts index 7bc0c2194d91d..1a467b7661999 100644 --- a/packages/core/src/image_performance_warning.ts +++ b/packages/core/src/image_performance_warning.ts @@ -22,7 +22,6 @@ const SCAN_DELAY = 200; const OVERSIZED_IMAGE_TOLERANCE = 1200; - @Injectable({providedIn: 'root'}) export class ImagePerformanceWarning implements OnDestroy { // Map of full image URLs -> original `ngSrc` values. @@ -38,7 +37,8 @@ export class ImagePerformanceWarning implements OnDestroy { return; } this.observer = this.initPerformanceObserver(); - const win = getDocument().defaultView; + const doc = getDocument(); + const win = doc.defaultView; if (typeof win !== 'undefined') { this.window = win; // Wait to avoid race conditions where LCP image triggers @@ -49,7 +49,16 @@ export class ImagePerformanceWarning implements OnDestroy { // Angular doesn't have to run change detection whenever any asynchronous tasks are invoked in // the scope of this functionality. this.ngZone.runOutsideAngular(() => { - this.window?.addEventListener('load', waitToScan, {once: true}); + // Consider the case when the application is created and destroyed multiple times. + // Typically, applications are created instantly once the page is loaded, and the + // `window.load` listener is always triggered. However, the `window.load` event will never + // be fired if the page is loaded, and the application is created later. Checking for + // `readyState` is the easiest way to determine whether the page has been loaded or not. + if (doc.readyState === 'complete') { + waitToScan(); + } else { + this.window?.addEventListener('load', waitToScan, {once: true}); + } }); } } From 29c5416d14638a05a894269aa5dbe67e98754418 Mon Sep 17 00:00:00 2001 From: arturovt Date: Fri, 17 Nov 2023 11:53:39 +0200 Subject: [PATCH 08/19] fix(common): remove `load` on image once it fails to load (#52990) This commit adds an `error` listener to image elements and removes both `load` and `error` listeners once the image loads or fails to load. The `load` listener would never have been removed if the image failed to load. PR Close #52990 --- .../ng_optimized_image/ng_optimized_image.ts | 25 ++++++++++++++++--- 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/packages/common/src/directives/ng_optimized_image/ng_optimized_image.ts b/packages/common/src/directives/ng_optimized_image/ng_optimized_image.ts index 1e8aaabc7f093..27c2bf81cad11 100644 --- a/packages/common/src/directives/ng_optimized_image/ng_optimized_image.ts +++ b/packages/common/src/directives/ng_optimized_image/ng_optimized_image.ts @@ -746,8 +746,9 @@ function assertGreaterThanZero(dir: NgOptimizedImage, inputValue: unknown, input */ function assertNoImageDistortion( dir: NgOptimizedImage, img: HTMLImageElement, renderer: Renderer2) { - const removeListenerFn = renderer.listen(img, 'load', () => { - removeListenerFn(); + const removeLoadListenerFn = renderer.listen(img, 'load', () => { + removeLoadListenerFn(); + removeErrorListenerFn(); const computedStyle = window.getComputedStyle(img); let renderedWidth = parseFloat(computedStyle.getPropertyValue('width')); let renderedHeight = parseFloat(computedStyle.getPropertyValue('height')); @@ -828,6 +829,15 @@ function assertNoImageDistortion( } } }); + + // We only listen to the `error` event to remove the `load` event listener because it will not be + // fired if the image fails to load. This is done to prevent memory leaks in development mode + // because image elements aren't garbage-collected properly. It happens because zone.js stores the + // event listener directly on the element and closures capture `dir`. + const removeErrorListenerFn = renderer.listen(img, 'error', () => { + removeLoadListenerFn(); + removeErrorListenerFn(); + }); } /** @@ -870,8 +880,9 @@ function assertEmptyWidthAndHeight(dir: NgOptimizedImage) { */ function assertNonZeroRenderedHeight( dir: NgOptimizedImage, img: HTMLImageElement, renderer: Renderer2) { - const removeListenerFn = renderer.listen(img, 'load', () => { - removeListenerFn(); + const removeLoadListenerFn = renderer.listen(img, 'load', () => { + removeLoadListenerFn(); + removeErrorListenerFn(); const renderedHeight = img.clientHeight; if (dir.fill && renderedHeight === 0) { console.warn(formatRuntimeError( @@ -883,6 +894,12 @@ function assertNonZeroRenderedHeight( `property defined and the height of the element is not zero.`)); } }); + + // See comments in the `assertNoImageDistortion`. + const removeErrorListenerFn = renderer.listen(img, 'error', () => { + removeLoadListenerFn(); + removeErrorListenerFn(); + }); } /** From e33f6e0f1a483cad908fa6d7376d62332797499c Mon Sep 17 00:00:00 2001 From: anthonyfr75 Date: Sun, 19 Nov 2023 18:28:59 +0100 Subject: [PATCH 09/19] fix(migrations): control flow migration fails for async pipe with unboxing of observable (#52756) (#52972) Update control flow syntax to use 'as' for proper async pipe handling, accounting for variable whitespace before 'let'. Fixes #52756 PR Close #52972 --- .../ng-generate/control-flow-migration/ifs.ts | 15 ++++- .../test/control_flow_migration_spec.ts | 64 ++++++++++++++++++- 2 files changed, 75 insertions(+), 4 deletions(-) diff --git a/packages/core/schematics/ng-generate/control-flow-migration/ifs.ts b/packages/core/schematics/ng-generate/control-flow-migration/ifs.ts index 34527b4a9b546..959623a4820bf 100644 --- a/packages/core/schematics/ng-generate/control-flow-migration/ifs.ts +++ b/packages/core/schematics/ng-generate/control-flow-migration/ifs.ts @@ -84,7 +84,10 @@ function migrateNgIf(etm: ElementToMigrate, tmpl: string, offset: number): Resul function buildIfBlock(etm: ElementToMigrate, tmpl: string, offset: number): Result { // includes the mandatory semicolon before as const lbString = etm.hasLineBreaks ? '\n' : ''; - const condition = etm.attr.value.replace(' as ', '; as '); + const condition = etm.attr.value + .replace(' as ', '; as ') + // replace 'let' with 'as' whatever spaces are between ; and 'let' + .replace(/;\s*let/g, '; as'); const originals = getOriginals(etm, tmpl, offset); @@ -106,7 +109,10 @@ function buildIfBlock(etm: ElementToMigrate, tmpl: string, offset: number): Resu function buildStandardIfElseBlock( etm: ElementToMigrate, tmpl: string, elseString: string, offset: number): Result { // includes the mandatory semicolon before as - const condition = etm.getCondition(elseString).replace(' as ', '; as '); + const condition = etm.getCondition(elseString) + .replace(' as ', '; as ') + // replace 'let' with 'as' whatever spaces are between ; and 'let' + .replace(/;\s*let/g, '; as'); const elsePlaceholder = `#${etm.getTemplateName(elseString)}|`; return buildIfElseBlock(etm, tmpl, condition, elsePlaceholder, offset); } @@ -151,7 +157,10 @@ function buildStandardIfThenElseBlock( etm: ElementToMigrate, tmpl: string, thenString: string, elseString: string, offset: number): Result { // includes the mandatory semicolon before as - const condition = etm.getCondition(thenString).replace(' as ', '; as '); + const condition = etm.getCondition(thenString) + .replace(' as ', '; as ') + // replace 'let' with 'as' whatever spaces are between ; and 'let' + .replace(/;\s*let/g, '; as'); const thenPlaceholder = `#${etm.getTemplateName(thenString, elseString)}|`; const elsePlaceholder = `#${etm.getTemplateName(elseString)}|`; return buildIfThenElseBlock(etm, tmpl, condition, thenPlaceholder, elsePlaceholder, offset); diff --git a/packages/core/schematics/test/control_flow_migration_spec.ts b/packages/core/schematics/test/control_flow_migration_spec.ts index 5452e32edae20..c06e74cd898ef 100644 --- a/packages/core/schematics/test/control_flow_migration_spec.ts +++ b/packages/core/schematics/test/control_flow_migration_spec.ts @@ -100,7 +100,6 @@ describe('control flow migration', () => { } catch (e: any) { error = e.message; } - expect(error).toBe('Cannot run control flow migration outside of the current project.'); }); @@ -3513,5 +3512,68 @@ describe('control flow migration', () => { expect(content).toContain( 'template: `
@if (toggle) {
@if (show) {shrug}
}
`'); }); + + it('should update let value in a build if block to as value for the new control flow', + async () => { + writeFile('/comp.ts', ` + import {Component} from '@angular/core'; + import {NgIf} from '@angular/common'; + + @Component({ + imports: [NgIf], + template: \` {{value}} \` + }) + class Comp { + value$ = of('Rica'); + } + `); + + await runMigration(); + const content = tree.readContent('/comp.ts'); + expect(content).toContain('template: `@if (value$ | async; as value) { {{value}} }`'); + }); + + it('should update let value in a standard if else block to as value for the new control flow', + async () => { + writeFile('/comp.ts', ` + import {Component} from '@angular/core'; + import {CommonModule} from '@angular/common'; + + @Component({ + imports: [CommonModule], + template: \`
Hello {{ userName }} !
\` + }) + class Comp { + logStatus$ = of({ name: 'Robert' }); + } + `); + + await runMigration(); + const content = tree.readContent('/comp.ts'); + expect(content).toContain( + 'template: `@if ((logStatus$ | async)?.name; as userName) {
Hello {{ userName }} !
} @else {}`'); + }); + + it('should update let value in a standard if else, then block to as value for the new control flow', + async () => { + writeFile('/comp.ts', ` + import {Component} from '@angular/core'; + import {CommonModule} from '@angular/common'; + + @Component({ + imports: [CommonModule], + template: \`
Log In + Log Out\` + }) + class Comp { + isLoggedIn$ = of(true); + } + `); + + await runMigration(); + const content = tree.readContent('/comp.ts'); + expect(content).toContain( + 'template: `@if (isLoggedIn$ | async; as logIn) {\n Log In\n} @else {\n Log Out\n}`'); + }); }); }); From 5fb707f81aee43751e61d2ed0861afc9b85bc85a Mon Sep 17 00:00:00 2001 From: Kristiyan Kostadinov Date: Thu, 16 Nov 2023 12:52:23 +0100 Subject: [PATCH 10/19] fix(compiler): produce placeholder for blocks in i18n bundles (#52958) When blocks were initially implemented, they were represented as containers in the i18n AST. This is problematic, because block affect the structure of the message. These changes introduce a new `BlockPlaceholder` AST node and integrate it into the i18n pipeline. With the new node blocks are represented with the `START_BLOCK_` and `CLOSE_BLOCK_` placeholders. PR Close #52958 --- packages/compiler/src/compiler.ts | 3 +- .../compiler/src/expression_parser/parser.ts | 2 +- packages/compiler/src/i18n/digest.ts | 5 ++ .../compiler/src/i18n/extractor_merger.ts | 5 +- packages/compiler/src/i18n/i18n_ast.ts | 28 ++++++++++ .../compiler/src/i18n/i18n_html_parser.ts | 2 +- packages/compiler/src/i18n/i18n_parser.ts | 40 +++++++++++--- packages/compiler/src/i18n/message_bundle.ts | 12 ++++- .../src/i18n/serializers/placeholder.ts | 36 +++++++++++++ .../src/i18n/serializers/serializer.ts | 6 +++ .../compiler/src/i18n/serializers/xliff.ts | 9 ++++ .../compiler/src/i18n/serializers/xliff2.ts | 19 +++++++ packages/compiler/src/i18n/serializers/xmb.ts | 14 +++++ .../compiler/src/i18n/translation_bundle.ts | 6 +++ packages/compiler/src/jit_compiler_facade.ts | 2 +- packages/compiler/src/ml_parser/ast.ts | 11 ++-- .../{interpolation_config.ts => defaults.ts} | 2 + packages/compiler/src/ml_parser/lexer.ts | 2 +- .../compiler/src/render3/partial/component.ts | 2 +- packages/compiler/src/render3/view/api.ts | 2 +- .../src/render3/view/i18n/get_msg_utils.ts | 5 ++ .../src/render3/view/i18n/icu_serializer.ts | 5 ++ .../src/render3/view/i18n/localize_utils.ts | 7 +++ .../compiler/src/render3/view/i18n/meta.ts | 10 ++-- .../compiler/src/render3/view/template.ts | 2 +- .../phases/resolve_i18n_icu_placeholders.ts | 18 +++++-- .../src/template_parser/binding_parser.ts | 4 +- .../test/expression_parser/utils/unparser.ts | 2 +- .../test/i18n/extractor_merger_spec.ts | 8 ++- packages/compiler/test/i18n/i18n_ast_spec.ts | 14 ++++- .../compiler/test/i18n/i18n_parser_spec.ts | 2 +- .../compiler/test/i18n/integration_common.ts | 2 +- .../compiler/test/i18n/message_bundle_spec.ts | 2 +- .../test/i18n/serializers/placeholder_spec.ts | 12 +++++ .../test/i18n/serializers/xliff2_spec.ts | 52 +++++++++++++------ .../test/i18n/serializers/xliff_spec.ts | 50 ++++++++++++------ .../test/i18n/serializers/xmb_spec.ts | 6 ++- packages/compiler/test/render3/view/util.ts | 2 +- 38 files changed, 333 insertions(+), 78 deletions(-) rename packages/compiler/src/ml_parser/{interpolation_config.ts => defaults.ts} (92%) diff --git a/packages/compiler/src/compiler.ts b/packages/compiler/src/compiler.ts index 3b315da5f7d9b..7b6c8508fe0d4 100644 --- a/packages/compiler/src/compiler.ts +++ b/packages/compiler/src/compiler.ts @@ -38,7 +38,7 @@ export * from './version'; export {CompilerConfig, preserveWhitespacesDefault} from './config'; export * from './resource_loader'; export {ConstantPool} from './constant_pool'; -export {DEFAULT_INTERPOLATION_CONFIG, InterpolationConfig} from './ml_parser/interpolation_config'; +export {DEFAULT_INTERPOLATION_CONFIG, InterpolationConfig} from './ml_parser/defaults'; export * from './schema/element_schema_registry'; export * from './i18n/index'; export * from './expression_parser/ast'; @@ -47,7 +47,6 @@ export * from './expression_parser/parser'; export * from './ml_parser/ast'; export * from './ml_parser/html_parser'; export * from './ml_parser/html_tags'; -export * from './ml_parser/interpolation_config'; export * from './ml_parser/tags'; export {ParseTreeResult, TreeError} from './ml_parser/parser'; export {LexerRange} from './ml_parser/lexer'; diff --git a/packages/compiler/src/expression_parser/parser.ts b/packages/compiler/src/expression_parser/parser.ts index a83db2e26b4ce..a60c08efd84cf 100644 --- a/packages/compiler/src/expression_parser/parser.ts +++ b/packages/compiler/src/expression_parser/parser.ts @@ -7,7 +7,7 @@ */ import * as chars from '../chars'; -import {DEFAULT_INTERPOLATION_CONFIG, InterpolationConfig} from '../ml_parser/interpolation_config'; +import {DEFAULT_INTERPOLATION_CONFIG, InterpolationConfig} from '../ml_parser/defaults'; import {InterpolatedAttributeToken, InterpolatedTextToken, TokenType as MlParserTokenType} from '../ml_parser/tokens'; import {AbsoluteSourceSpan, AST, ASTWithSource, Binary, BindingPipe, Call, Chain, Conditional, EmptyExpr, ExpressionBinding, ImplicitReceiver, Interpolation, KeyedRead, KeyedWrite, LiteralArray, LiteralMap, LiteralMapKey, LiteralPrimitive, NonNullAssert, ParserError, ParseSpan, PrefixNot, PropertyRead, PropertyWrite, RecursiveAstVisitor, SafeCall, SafeKeyedRead, SafePropertyRead, TemplateBinding, TemplateBindingIdentifier, ThisReceiver, Unary, VariableBinding} from './ast'; diff --git a/packages/compiler/src/i18n/digest.ts b/packages/compiler/src/i18n/digest.ts index 276515c609844..68b8c18e4e0bd 100644 --- a/packages/compiler/src/i18n/digest.ts +++ b/packages/compiler/src/i18n/digest.ts @@ -81,6 +81,11 @@ class _SerializerVisitor implements i18n.Visitor { visitIcuPlaceholder(ph: i18n.IcuPlaceholder, context?: any): any { return `${ph.value.visit(this)}`; } + + visitBlockPlaceholder(ph: i18n.BlockPlaceholder, context: any): any { + return `${ + ph.children.map(child => child.visit(this)).join(', ')}`; + } } const serializerVisitor = new _SerializerVisitor(); diff --git a/packages/compiler/src/i18n/extractor_merger.ts b/packages/compiler/src/i18n/extractor_merger.ts index 264427b7827c1..33f951c66963c 100644 --- a/packages/compiler/src/i18n/extractor_merger.ts +++ b/packages/compiler/src/i18n/extractor_merger.ts @@ -7,7 +7,7 @@ */ import * as html from '../ml_parser/ast'; -import {InterpolationConfig} from '../ml_parser/interpolation_config'; +import {DEFAULT_CONTAINER_BLOCKS, InterpolationConfig} from '../ml_parser/defaults'; import {ParseTreeResult} from '../ml_parser/parser'; import * as i18n from './i18n_ast'; @@ -308,7 +308,8 @@ class _Visitor implements html.Visitor { this._errors = []; this._messages = []; this._inImplicitNode = false; - this._createI18nMessage = createI18nMessageFactory(interpolationConfig); + this._createI18nMessage = + createI18nMessageFactory(interpolationConfig, DEFAULT_CONTAINER_BLOCKS); } // looks for translatable attributes diff --git a/packages/compiler/src/i18n/i18n_ast.ts b/packages/compiler/src/i18n/i18n_ast.ts index a8b5f77d16d36..94d86128d78d8 100644 --- a/packages/compiler/src/i18n/i18n_ast.ts +++ b/packages/compiler/src/i18n/i18n_ast.ts @@ -129,6 +129,17 @@ export class IcuPlaceholder implements Node { } } +export class BlockPlaceholder implements Node { + constructor( + public name: string, public parameters: string[], public startName: string, + public closeName: string, public children: Node[], public sourceSpan: ParseSourceSpan, + public startSourceSpan: ParseSourceSpan|null, public endSourceSpan: ParseSourceSpan|null) {} + + visit(visitor: Visitor, context?: any): any { + return visitor.visitBlockPlaceholder(this, context); + } +} + /** * Each HTML node that is affect by an i18n tag will also have an `i18n` property that is of type * `I18nMeta`. @@ -144,6 +155,7 @@ export interface Visitor { visitTagPlaceholder(ph: TagPlaceholder, context?: any): any; visitPlaceholder(ph: Placeholder, context?: any): any; visitIcuPlaceholder(ph: IcuPlaceholder, context?: any): any; + visitBlockPlaceholder(ph: BlockPlaceholder, context?: any): any; } // Clone the AST @@ -178,6 +190,13 @@ export class CloneVisitor implements Visitor { visitIcuPlaceholder(ph: IcuPlaceholder, context?: any): IcuPlaceholder { return new IcuPlaceholder(ph.value, ph.name, ph.sourceSpan); } + + visitBlockPlaceholder(ph: BlockPlaceholder, context?: any): BlockPlaceholder { + const children = ph.children.map(n => n.visit(this, context)); + return new BlockPlaceholder( + ph.name, ph.parameters, ph.startName, ph.closeName, children, ph.sourceSpan, + ph.startSourceSpan, ph.endSourceSpan); + } } // Visit all the nodes recursively @@ -201,6 +220,10 @@ export class RecurseVisitor implements Visitor { visitPlaceholder(ph: Placeholder, context?: any): any {} visitIcuPlaceholder(ph: IcuPlaceholder, context?: any): any {} + + visitBlockPlaceholder(ph: BlockPlaceholder, context?: any): any { + ph.children.forEach(child => child.visit(this)); + } } @@ -240,4 +263,9 @@ class LocalizeMessageStringVisitor implements Visitor { visitIcuPlaceholder(ph: IcuPlaceholder): any { return `{$${ph.name}}`; } + + visitBlockPlaceholder(ph: BlockPlaceholder): any { + const children = ph.children.map(child => child.visit(this)).join(''); + return `{$${ph.startName}}${children}{$${ph.closeName}}`; + } } diff --git a/packages/compiler/src/i18n/i18n_html_parser.ts b/packages/compiler/src/i18n/i18n_html_parser.ts index 41029bfb8e937..cf54abd543918 100644 --- a/packages/compiler/src/i18n/i18n_html_parser.ts +++ b/packages/compiler/src/i18n/i18n_html_parser.ts @@ -7,8 +7,8 @@ */ import {MissingTranslationStrategy} from '../core'; +import {DEFAULT_INTERPOLATION_CONFIG} from '../ml_parser/defaults'; import {HtmlParser} from '../ml_parser/html_parser'; -import {DEFAULT_INTERPOLATION_CONFIG} from '../ml_parser/interpolation_config'; import {TokenizeOptions} from '../ml_parser/lexer'; import {ParseTreeResult} from '../ml_parser/parser'; import {Console} from '../util'; diff --git a/packages/compiler/src/i18n/i18n_parser.ts b/packages/compiler/src/i18n/i18n_parser.ts index c5ef54aa94459..ee2977751ec26 100644 --- a/packages/compiler/src/i18n/i18n_parser.ts +++ b/packages/compiler/src/i18n/i18n_parser.ts @@ -9,8 +9,8 @@ import {Lexer as ExpressionLexer} from '../expression_parser/lexer'; import {Parser as ExpressionParser} from '../expression_parser/parser'; import * as html from '../ml_parser/ast'; +import {InterpolationConfig} from '../ml_parser/defaults'; import {getHtmlTagDefinition} from '../ml_parser/html_tags'; -import {InterpolationConfig} from '../ml_parser/interpolation_config'; import {InterpolatedAttributeToken, InterpolatedTextToken, TokenType} from '../ml_parser/tokens'; import {ParseSourceSpan} from '../parse_util'; @@ -29,9 +29,9 @@ export interface I18nMessageFactory { /** * Returns a function converting html nodes to an i18n Message given an interpolationConfig */ -export function createI18nMessageFactory(interpolationConfig: InterpolationConfig): - I18nMessageFactory { - const visitor = new _I18nVisitor(_expParser, interpolationConfig); +export function createI18nMessageFactory( + interpolationConfig: InterpolationConfig, containerBlocks: Set): I18nMessageFactory { + const visitor = new _I18nVisitor(_expParser, interpolationConfig, containerBlocks); return (nodes, meaning, description, customId, visitNodeFn) => visitor.toI18nMessage(nodes, meaning, description, customId, visitNodeFn); } @@ -52,7 +52,9 @@ function noopVisitNodeFn(_html: html.Node, i18n: i18n.Node): i18n.Node { class _I18nVisitor implements html.Visitor { constructor( private _expressionParser: ExpressionParser, - private _interpolationConfig: InterpolationConfig) {} + private _interpolationConfig: InterpolationConfig, + private _containerBlocks: Set, + ) {} public toI18nMessage( nodes: html.Node[], meaning = '', description = '', customId = '', @@ -164,11 +166,35 @@ class _I18nVisitor implements html.Visitor { visitBlock(block: html.Block, context: I18nMessageVisitorContext) { const children = html.visitAll(this, block.children, context); - const node = new i18n.Container(children, block.sourceSpan); + + if (this._containerBlocks.has(block.name)) { + return new i18n.Container(children, block.sourceSpan); + } + + const parameters = block.parameters.map(param => param.expression); + const startPhName = + context.placeholderRegistry.getStartBlockPlaceholderName(block.name, parameters); + const closePhName = context.placeholderRegistry.getCloseBlockPlaceholderName(block.name); + + context.placeholderToContent[startPhName] = { + text: block.startSourceSpan.toString(), + sourceSpan: block.startSourceSpan, + }; + + context.placeholderToContent[closePhName] = { + text: block.endSourceSpan ? block.endSourceSpan.toString() : '}', + sourceSpan: block.endSourceSpan ?? block.sourceSpan, + }; + + const node = new i18n.BlockPlaceholder( + block.name, parameters, startPhName, closePhName, children, block.sourceSpan, + block.startSourceSpan, block.endSourceSpan); return context.visitNodeFn(block, node); } - visitBlockParameter(_parameter: html.BlockParameter, _context: I18nMessageVisitorContext) {} + visitBlockParameter(_parameter: html.BlockParameter, _context: I18nMessageVisitorContext) { + throw new Error('Unreachable code'); + } /** * Convert, text and interpolated tokens up into text and placeholder pieces. diff --git a/packages/compiler/src/i18n/message_bundle.ts b/packages/compiler/src/i18n/message_bundle.ts index 3bf6d90a98493..070452cd8282a 100644 --- a/packages/compiler/src/i18n/message_bundle.ts +++ b/packages/compiler/src/i18n/message_bundle.ts @@ -6,8 +6,8 @@ * found in the LICENSE file at https://angular.io/license */ +import {InterpolationConfig} from '../ml_parser/defaults'; import {HtmlParser} from '../ml_parser/html_parser'; -import {InterpolationConfig} from '../ml_parser/interpolation_config'; import {ParseError} from '../parse_util'; import {extractMessages} from './extractor_merger'; @@ -99,6 +99,16 @@ class MapPlaceholderNames extends i18n.CloneVisitor { ph.startSourceSpan, ph.endSourceSpan); } + override visitBlockPlaceholder(ph: i18n.BlockPlaceholder, mapper: PlaceholderMapper): + i18n.BlockPlaceholder { + const startName = mapper.toPublicName(ph.startName)!; + const closeName = ph.closeName ? mapper.toPublicName(ph.closeName)! : ph.closeName; + const children = ph.children.map(n => n.visit(this, mapper)); + return new i18n.BlockPlaceholder( + ph.name, ph.parameters, startName, closeName, children, ph.sourceSpan, ph.startSourceSpan, + ph.endSourceSpan); + } + override visitPlaceholder(ph: i18n.Placeholder, mapper: PlaceholderMapper): i18n.Placeholder { return new i18n.Placeholder(ph.value, mapper.toPublicName(ph.name)!, ph.sourceSpan); } diff --git a/packages/compiler/src/i18n/serializers/placeholder.ts b/packages/compiler/src/i18n/serializers/placeholder.ts index 17f4b72dbf67c..55831fbf6f3e9 100644 --- a/packages/compiler/src/i18n/serializers/placeholder.ts +++ b/packages/compiler/src/i18n/serializers/placeholder.ts @@ -97,6 +97,29 @@ export class PlaceholderRegistry { return this._generateUniqueName(name.toUpperCase()); } + getStartBlockPlaceholderName(name: string, parameters: string[]): string { + const signature = this._hashBlock(name, parameters); + if (this._signatureToName[signature]) { + return this._signatureToName[signature]; + } + + const placeholder = this._generateUniqueName(`START_BLOCK_${this._toSnakeCase(name)}`); + this._signatureToName[signature] = placeholder; + return placeholder; + } + + getCloseBlockPlaceholderName(name: string): string { + const signature = this._hashClosingBlock(name); + if (this._signatureToName[signature]) { + return this._signatureToName[signature]; + } + + const placeholder = this._generateUniqueName(`CLOSE_BLOCK_${this._toSnakeCase(name)}`); + this._signatureToName[signature] = placeholder; + return placeholder; + } + + // Generate a hash for a tag - does not take attribute order into account private _hashTag(tag: string, attrs: {[k: string]: string}, isVoid: boolean): string { const start = `<${tag}`; @@ -110,6 +133,19 @@ export class PlaceholderRegistry { return this._hashTag(`/${tag}`, {}, false); } + private _hashBlock(name: string, parameters: string[]): string { + const params = parameters.length === 0 ? '' : ` (${parameters.sort().join('; ')})`; + return `@${name}${params} {}`; + } + + private _hashClosingBlock(name: string): string { + return this._hashBlock(`close_${name}`, []); + } + + private _toSnakeCase(name: string) { + return name.toUpperCase().replace(/[^A-Z0-9]/g, '_'); + } + private _generateUniqueName(base: string): string { const seen = this._placeHolderNameCounts.hasOwnProperty(base); if (!seen) { diff --git a/packages/compiler/src/i18n/serializers/serializer.ts b/packages/compiler/src/i18n/serializers/serializer.ts index effcfa6091f4f..e7a4090545d8a 100644 --- a/packages/compiler/src/i18n/serializers/serializer.ts +++ b/packages/compiler/src/i18n/serializers/serializer.ts @@ -77,6 +77,12 @@ export class SimplePlaceholderMapper extends i18n.RecurseVisitor implements Plac this.visitPlaceholderName(ph.name); } + override visitBlockPlaceholder(ph: i18n.BlockPlaceholder, context?: any): any { + this.visitPlaceholderName(ph.startName); + super.visitBlockPlaceholder(ph, context); + this.visitPlaceholderName(ph.closeName); + } + override visitIcuPlaceholder(ph: i18n.IcuPlaceholder, context?: any): any { this.visitPlaceholderName(ph.name); } diff --git a/packages/compiler/src/i18n/serializers/xliff.ts b/packages/compiler/src/i18n/serializers/xliff.ts index 7c9b5a1a5dc38..54aabbd252c45 100644 --- a/packages/compiler/src/i18n/serializers/xliff.ts +++ b/packages/compiler/src/i18n/serializers/xliff.ts @@ -164,6 +164,15 @@ class _WriteVisitor implements i18n.Visitor { return [new xml.Tag(_PLACEHOLDER_TAG, {id: ph.name, 'equiv-text': `{{${ph.value}}}`})]; } + visitBlockPlaceholder(ph: i18n.BlockPlaceholder, context?: any): xml.Node[] { + const ctype = `x-${ph.name.toLowerCase().replace(/[^a-z0-9]/g, '-')}`; + const startTagPh = + new xml.Tag(_PLACEHOLDER_TAG, {id: ph.startName, ctype, 'equiv-text': `@${ph.name}`}); + const closeTagPh = new xml.Tag(_PLACEHOLDER_TAG, {id: ph.closeName, ctype, 'equiv-text': `}`}); + + return [startTagPh, ...this.serialize(ph.children), closeTagPh]; + } + visitIcuPlaceholder(ph: i18n.IcuPlaceholder, context?: any): xml.Node[] { const equivText = `{${ph.value.expression}, ${ph.value.type}, ${ Object.keys(ph.value.cases).map((value: string) => value + ' {...}').join(' ')}}`; diff --git a/packages/compiler/src/i18n/serializers/xliff2.ts b/packages/compiler/src/i18n/serializers/xliff2.ts index 370a055c8663a..c6ba2f12cd5de 100644 --- a/packages/compiler/src/i18n/serializers/xliff2.ts +++ b/packages/compiler/src/i18n/serializers/xliff2.ts @@ -178,6 +178,25 @@ class _WriteVisitor implements i18n.Visitor { })]; } + visitBlockPlaceholder(ph: i18n.BlockPlaceholder, context?: any): xml.Node[] { + const tagPc = new xml.Tag(_PLACEHOLDER_SPANNING_TAG, { + id: (this._nextPlaceholderId++).toString(), + equivStart: ph.startName, + equivEnd: ph.closeName, + type: 'other', + dispStart: `@${ph.name}`, + dispEnd: `}`, + }); + const nodes: xml.Node[] = [].concat(...ph.children.map(node => node.visit(this))); + if (nodes.length) { + nodes.forEach((node: xml.Node) => tagPc.children.push(node)); + } else { + tagPc.children.push(new xml.Text('')); + } + + return [tagPc]; + } + visitIcuPlaceholder(ph: i18n.IcuPlaceholder, context?: any): xml.Node[] { const cases = Object.keys(ph.value.cases).map((value: string) => value + ' {...}').join(' '); const idStr = (this._nextPlaceholderId++).toString(); diff --git a/packages/compiler/src/i18n/serializers/xmb.ts b/packages/compiler/src/i18n/serializers/xmb.ts index 3688454d308d4..28bb0974716e4 100644 --- a/packages/compiler/src/i18n/serializers/xmb.ts +++ b/packages/compiler/src/i18n/serializers/xmb.ts @@ -148,6 +148,20 @@ class _Visitor implements i18n.Visitor { ]; } + visitBlockPlaceholder(ph: i18n.BlockPlaceholder, context?: any): xml.Node[] { + const startAsText = new xml.Text(`@${ph.name}`); + const startEx = new xml.Tag(_EXAMPLE_TAG, {}, [startAsText]); + // TC requires PH to have a non empty EX, and uses the text node to show the "original" value. + const startTagPh = new xml.Tag(_PLACEHOLDER_TAG, {name: ph.startName}, [startEx, startAsText]); + + const closeAsText = new xml.Text(`}`); + const closeEx = new xml.Tag(_EXAMPLE_TAG, {}, [closeAsText]); + // TC requires PH to have a non empty EX, and uses the text node to show the "original" value. + const closeTagPh = new xml.Tag(_PLACEHOLDER_TAG, {name: ph.closeName}, [closeEx, closeAsText]); + + return [startTagPh, ...this.serialize(ph.children), closeTagPh]; + } + visitIcuPlaceholder(ph: i18n.IcuPlaceholder, context?: any): xml.Node[] { const icuExpression = ph.value.expression; const icuType = ph.value.type; diff --git a/packages/compiler/src/i18n/translation_bundle.ts b/packages/compiler/src/i18n/translation_bundle.ts index 6d5b19be4b9af..f6cd6e305a391 100644 --- a/packages/compiler/src/i18n/translation_bundle.ts +++ b/packages/compiler/src/i18n/translation_bundle.ts @@ -149,6 +149,12 @@ class I18nToHtmlVisitor implements i18n.Visitor { return this._convertToText(this._srcMsg.placeholderToMessage[ph.name]); } + visitBlockPlaceholder(ph: i18n.BlockPlaceholder, context?: any): string { + const params = ph.parameters.length === 0 ? '' : ` (${ph.parameters.join('; ')})`; + const children = ph.children.map((c: i18n.Node) => c.visit(this)).join(''); + return `@${ph.name}${params} {${children}}`; + } + /** * Convert a source message to a translated text string: * - text nodes are replaced with their translation, diff --git a/packages/compiler/src/jit_compiler_facade.ts b/packages/compiler/src/jit_compiler_facade.ts index 42c6559bc1047..fb4dd7b02c7be 100644 --- a/packages/compiler/src/jit_compiler_facade.ts +++ b/packages/compiler/src/jit_compiler_facade.ts @@ -10,7 +10,7 @@ import {CompilerFacade, CoreEnvironment, ExportedCompilerFacade, InputMap, Input import {ConstantPool} from './constant_pool'; import {ChangeDetectionStrategy, HostBinding, HostListener, Input, Output, ViewEncapsulation} from './core'; import {compileInjectable} from './injectable_compiler_2'; -import {DEFAULT_INTERPOLATION_CONFIG, InterpolationConfig} from './ml_parser/interpolation_config'; +import {DEFAULT_INTERPOLATION_CONFIG, InterpolationConfig} from './ml_parser/defaults'; import {DeclareVarStmt, Expression, literal, LiteralExpr, Statement, StmtModifier, WrappedNodeExpr} from './output/output_ast'; import {JitEvaluator} from './output/output_jit'; import {ParseError, ParseSourceSpan, r3JitTypeSourceSpan} from './parse_util'; diff --git a/packages/compiler/src/ml_parser/ast.ts b/packages/compiler/src/ml_parser/ast.ts index 1d608a28861a4..b509728e408a5 100644 --- a/packages/compiler/src/ml_parser/ast.ts +++ b/packages/compiler/src/ml_parser/ast.ts @@ -86,13 +86,16 @@ export class Comment implements BaseNode { } } -export class Block implements BaseNode { +export class Block extends NodeWithI18n { constructor( public name: string, public parameters: BlockParameter[], public children: Node[], - public sourceSpan: ParseSourceSpan, public nameSpan: ParseSourceSpan, - public startSourceSpan: ParseSourceSpan, public endSourceSpan: ParseSourceSpan|null = null) {} + sourceSpan: ParseSourceSpan, public nameSpan: ParseSourceSpan, + public startSourceSpan: ParseSourceSpan, public endSourceSpan: ParseSourceSpan|null = null, + i18n?: I18nMeta) { + super(sourceSpan, i18n); + } - visit(visitor: Visitor, context: any) { + override visit(visitor: Visitor, context: any) { return visitor.visitBlock(this, context); } } diff --git a/packages/compiler/src/ml_parser/interpolation_config.ts b/packages/compiler/src/ml_parser/defaults.ts similarity index 92% rename from packages/compiler/src/ml_parser/interpolation_config.ts rename to packages/compiler/src/ml_parser/defaults.ts index 70854b9f8de0e..90078a26a8e45 100644 --- a/packages/compiler/src/ml_parser/interpolation_config.ts +++ b/packages/compiler/src/ml_parser/defaults.ts @@ -23,3 +23,5 @@ export class InterpolationConfig { export const DEFAULT_INTERPOLATION_CONFIG: InterpolationConfig = new InterpolationConfig('{{', '}}'); + +export const DEFAULT_CONTAINER_BLOCKS = new Set(['switch']); diff --git a/packages/compiler/src/ml_parser/lexer.ts b/packages/compiler/src/ml_parser/lexer.ts index f2842341809ff..74ef678dea6b1 100644 --- a/packages/compiler/src/ml_parser/lexer.ts +++ b/packages/compiler/src/ml_parser/lexer.ts @@ -9,8 +9,8 @@ import * as chars from '../chars'; import {ParseError, ParseLocation, ParseSourceFile, ParseSourceSpan} from '../parse_util'; +import {DEFAULT_INTERPOLATION_CONFIG, InterpolationConfig} from './defaults'; import {NAMED_ENTITIES} from './entities'; -import {DEFAULT_INTERPOLATION_CONFIG, InterpolationConfig} from './interpolation_config'; import {TagContentType, TagDefinition} from './tags'; import {IncompleteTagOpenToken, TagOpenStartToken, Token, TokenType} from './tokens'; diff --git a/packages/compiler/src/render3/partial/component.ts b/packages/compiler/src/render3/partial/component.ts index 23913ed141eb6..c1f89aef9a80f 100644 --- a/packages/compiler/src/render3/partial/component.ts +++ b/packages/compiler/src/render3/partial/component.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ import * as core from '../../core'; -import {DEFAULT_INTERPOLATION_CONFIG} from '../../ml_parser/interpolation_config'; +import {DEFAULT_INTERPOLATION_CONFIG} from '../../ml_parser/defaults'; import * as o from '../../output/output_ast'; import {ParseLocation, ParseSourceFile, ParseSourceSpan} from '../../parse_util'; import {RecursiveVisitor, visitAll} from '../r3_ast'; diff --git a/packages/compiler/src/render3/view/api.ts b/packages/compiler/src/render3/view/api.ts index 74363df18e8d7..103eb707f7067 100644 --- a/packages/compiler/src/render3/view/api.ts +++ b/packages/compiler/src/render3/view/api.ts @@ -7,7 +7,7 @@ */ import {ChangeDetectionStrategy, ViewEncapsulation} from '../../core'; -import {InterpolationConfig} from '../../ml_parser/interpolation_config'; +import {InterpolationConfig} from '../../ml_parser/defaults'; import * as o from '../../output/output_ast'; import {ParseSourceSpan} from '../../parse_util'; import * as t from '../r3_ast'; diff --git a/packages/compiler/src/render3/view/i18n/get_msg_utils.ts b/packages/compiler/src/render3/view/i18n/get_msg_utils.ts index f17b5ac0fbf79..2a38d2eaa7f57 100644 --- a/packages/compiler/src/render3/view/i18n/get_msg_utils.ts +++ b/packages/compiler/src/render3/view/i18n/get_msg_utils.ts @@ -128,6 +128,11 @@ class GetMsgSerializerVisitor implements i18n.Visitor { return this.formatPh(ph.name); } + visitBlockPlaceholder(ph: i18n.BlockPlaceholder): any { + return `${this.formatPh(ph.startName)}${ph.children.map(child => child.visit(this)).join('')}${ + this.formatPh(ph.closeName)}`; + } + visitIcuPlaceholder(ph: i18n.IcuPlaceholder, context?: any): any { return this.formatPh(ph.name); } diff --git a/packages/compiler/src/render3/view/i18n/icu_serializer.ts b/packages/compiler/src/render3/view/i18n/icu_serializer.ts index 2466c7bfb393b..0454c145e74ec 100644 --- a/packages/compiler/src/render3/view/i18n/icu_serializer.ts +++ b/packages/compiler/src/render3/view/i18n/icu_serializer.ts @@ -37,6 +37,11 @@ class IcuSerializerVisitor implements i18n.Visitor { return this.formatPh(ph.name); } + visitBlockPlaceholder(ph: i18n.BlockPlaceholder): any { + return `${this.formatPh(ph.startName)}${ph.children.map(child => child.visit(this)).join('')}${ + this.formatPh(ph.closeName)}`; + } + visitIcuPlaceholder(ph: i18n.IcuPlaceholder, context?: any): any { return this.formatPh(ph.name); } diff --git a/packages/compiler/src/render3/view/i18n/localize_utils.ts b/packages/compiler/src/render3/view/i18n/localize_utils.ts index 436e353dd57ff..70006561ba808 100644 --- a/packages/compiler/src/render3/view/i18n/localize_utils.ts +++ b/packages/compiler/src/render3/view/i18n/localize_utils.ts @@ -68,6 +68,13 @@ class LocalizeSerializerVisitor implements i18n.Visitor { this.pieces.push(this.createPlaceholderPiece(ph.name, ph.sourceSpan)); } + visitBlockPlaceholder(ph: i18n.BlockPlaceholder): any { + this.pieces.push( + this.createPlaceholderPiece(ph.startName, ph.startSourceSpan ?? ph.sourceSpan)); + ph.children.forEach(child => child.visit(this)); + this.pieces.push(this.createPlaceholderPiece(ph.closeName, ph.endSourceSpan ?? ph.sourceSpan)); + } + visitIcuPlaceholder(ph: i18n.IcuPlaceholder): any { this.pieces.push( this.createPlaceholderPiece(ph.name, ph.sourceSpan, this.placeholderToMessage[ph.name])); diff --git a/packages/compiler/src/render3/view/i18n/meta.ts b/packages/compiler/src/render3/view/i18n/meta.ts index f70bb498cd8a8..30c4943545253 100644 --- a/packages/compiler/src/render3/view/i18n/meta.ts +++ b/packages/compiler/src/render3/view/i18n/meta.ts @@ -8,10 +8,10 @@ import {computeDecimalDigest, computeDigest, decimalDigest} from '../../../i18n/digest'; import * as i18n from '../../../i18n/i18n_ast'; -import {createI18nMessageFactory, I18nMessageFactory, VisitNodeFn} from '../../../i18n/i18n_parser'; +import {createI18nMessageFactory, VisitNodeFn} from '../../../i18n/i18n_parser'; import {I18nError} from '../../../i18n/parse_util'; import * as html from '../../../ml_parser/ast'; -import {DEFAULT_INTERPOLATION_CONFIG, InterpolationConfig} from '../../../ml_parser/interpolation_config'; +import {DEFAULT_CONTAINER_BLOCKS, DEFAULT_INTERPOLATION_CONFIG, InterpolationConfig} from '../../../ml_parser/defaults'; import {ParseTreeResult} from '../../../ml_parser/parser'; import * as o from '../../../output/output_ast'; import {isTrustedTypesSink} from '../../../schema/trusted_types_sinks'; @@ -53,13 +53,15 @@ export class I18nMetaVisitor implements html.Visitor { constructor( private interpolationConfig: InterpolationConfig = DEFAULT_INTERPOLATION_CONFIG, - private keepI18nAttrs = false, private enableI18nLegacyMessageIdFormat = false) {} + private keepI18nAttrs = false, private enableI18nLegacyMessageIdFormat = false, + private containerBlocks: Set = DEFAULT_CONTAINER_BLOCKS) {} private _generateI18nMessage( nodes: html.Node[], meta: string|i18n.I18nMeta = '', visitNodeFn?: VisitNodeFn): i18n.Message { const {meaning, description, customId} = this._parseMetadata(meta); - const createI18nMessage = createI18nMessageFactory(this.interpolationConfig); + const createI18nMessage = + createI18nMessageFactory(this.interpolationConfig, this.containerBlocks); const message = createI18nMessage(nodes, meaning, description, customId, visitNodeFn); this._setMessageId(message, meta); this._setLegacyIds(message, meta); diff --git a/packages/compiler/src/render3/view/template.ts b/packages/compiler/src/render3/view/template.ts index 3854cadb8329a..c320f471759d6 100644 --- a/packages/compiler/src/render3/view/template.ts +++ b/packages/compiler/src/render3/view/template.ts @@ -14,9 +14,9 @@ import {Lexer} from '../../expression_parser/lexer'; import {Parser} from '../../expression_parser/parser'; import * as i18n from '../../i18n/i18n_ast'; import * as html from '../../ml_parser/ast'; +import {DEFAULT_INTERPOLATION_CONFIG, InterpolationConfig} from '../../ml_parser/defaults'; import {HtmlParser} from '../../ml_parser/html_parser'; import {WhitespaceVisitor} from '../../ml_parser/html_whitespaces'; -import {DEFAULT_INTERPOLATION_CONFIG, InterpolationConfig} from '../../ml_parser/interpolation_config'; import {LexerRange} from '../../ml_parser/lexer'; import {isNgContainer as checkIsNgContainer, splitNsName} from '../../ml_parser/tags'; import {mapLiteral} from '../../output/map_util'; diff --git a/packages/compiler/src/template/pipeline/src/phases/resolve_i18n_icu_placeholders.ts b/packages/compiler/src/template/pipeline/src/phases/resolve_i18n_icu_placeholders.ts index 595660908f3e9..d9fdfa474087c 100644 --- a/packages/compiler/src/template/pipeline/src/phases/resolve_i18n_icu_placeholders.ts +++ b/packages/compiler/src/template/pipeline/src/phases/resolve_i18n_icu_placeholders.ts @@ -50,11 +50,9 @@ class ResolveIcuPlaceholdersVisitor extends i18n.RecurseVisitor { super(); } - override visitTagPlaceholder(placeholder: i18n.TagPlaceholder) { - super.visitTagPlaceholder(placeholder); - - // Add the start and end source span for tag placeholders. These need to be recorded for - // elements inside ICUs. The slots for the elements were recorded separately under the i18n + private visitContainerPlaceholder(placeholder: i18n.TagPlaceholder|i18n.BlockPlaceholder) { + // Add the start and end source span for container placeholders. These need to be recorded for + // elements inside ICUs. The slots for the nodes were recorded separately under the i18n // block's context as part of the `resolveI18nElementPlaceholders` phase. if (placeholder.startName && placeholder.startSourceSpan && !this.params.has(placeholder.startName)) { @@ -73,4 +71,14 @@ class ResolveIcuPlaceholdersVisitor extends i18n.RecurseVisitor { }]); } } + + override visitTagPlaceholder(placeholder: i18n.TagPlaceholder) { + super.visitTagPlaceholder(placeholder); + this.visitContainerPlaceholder(placeholder); + } + + override visitBlockPlaceholder(placeholder: i18n.BlockPlaceholder) { + super.visitBlockPlaceholder(placeholder); + this.visitContainerPlaceholder(placeholder); + } } diff --git a/packages/compiler/src/template_parser/binding_parser.ts b/packages/compiler/src/template_parser/binding_parser.ts index d640a9cf8eea3..d905e24cdd3ba 100644 --- a/packages/compiler/src/template_parser/binding_parser.ts +++ b/packages/compiler/src/template_parser/binding_parser.ts @@ -9,10 +9,10 @@ import {SecurityContext} from '../core'; import {AbsoluteSourceSpan, ASTWithSource, BindingPipe, BindingType, BoundElementProperty, EmptyExpr, ParsedEvent, ParsedEventType, ParsedProperty, ParsedPropertyType, ParsedVariable, ParserError, RecursiveAstVisitor, TemplateBinding, VariableBinding} from '../expression_parser/ast'; import {Parser} from '../expression_parser/parser'; -import {InterpolationConfig} from '../ml_parser/interpolation_config'; +import {InterpolationConfig} from '../ml_parser/defaults'; import {mergeNsAndName} from '../ml_parser/tags'; import {InterpolatedAttributeToken, InterpolatedTextToken} from '../ml_parser/tokens'; -import {ParseError, ParseErrorLevel, ParseLocation, ParseSourceSpan} from '../parse_util'; +import {ParseError, ParseErrorLevel, ParseSourceSpan} from '../parse_util'; import {ElementSchemaRegistry} from '../schema/element_schema_registry'; import {CssSelector} from '../selector'; import {splitAtColon, splitAtPeriod} from '../util'; diff --git a/packages/compiler/test/expression_parser/utils/unparser.ts b/packages/compiler/test/expression_parser/utils/unparser.ts index d4488498089f1..4cf474595f3dc 100644 --- a/packages/compiler/test/expression_parser/utils/unparser.ts +++ b/packages/compiler/test/expression_parser/utils/unparser.ts @@ -7,7 +7,7 @@ */ import {AST, AstVisitor, ASTWithSource, Binary, BindingPipe, Call, Chain, Conditional, ImplicitReceiver, Interpolation, KeyedRead, KeyedWrite, LiteralArray, LiteralMap, LiteralPrimitive, NonNullAssert, PrefixNot, PropertyRead, PropertyWrite, RecursiveAstVisitor, SafeCall, SafeKeyedRead, SafePropertyRead, ThisReceiver, Unary} from '../../../src/expression_parser/ast'; -import {DEFAULT_INTERPOLATION_CONFIG, InterpolationConfig} from '../../../src/ml_parser/interpolation_config'; +import {DEFAULT_INTERPOLATION_CONFIG, InterpolationConfig} from '../../../src/ml_parser/defaults'; class Unparser implements AstVisitor { private static _quoteRegExp = /"/g; diff --git a/packages/compiler/test/i18n/extractor_merger_spec.ts b/packages/compiler/test/i18n/extractor_merger_spec.ts index 3d0fa6df06101..27b7071c936fb 100644 --- a/packages/compiler/test/i18n/extractor_merger_spec.ts +++ b/packages/compiler/test/i18n/extractor_merger_spec.ts @@ -168,7 +168,13 @@ describe('Extractor', () => { it('should handle blocks inside of translated elements', () => { expect(extract('@if (cond) {main content} @else {else content}`')) - .toEqual([[['[main content]', ' ', '[else content]'], 'a', 'b|c', '']]); + .toEqual([[ + [ + 'main content', ' ', + 'else content' + ], + 'a', 'b|c', '' + ]]); }); }); diff --git a/packages/compiler/test/i18n/i18n_ast_spec.ts b/packages/compiler/test/i18n/i18n_ast_spec.ts index 0ef344da2a2c8..59933948afdb4 100644 --- a/packages/compiler/test/i18n/i18n_ast_spec.ts +++ b/packages/compiler/test/i18n/i18n_ast_spec.ts @@ -8,11 +8,12 @@ import {createI18nMessageFactory} from '../../src/i18n/i18n_parser'; import {Node} from '../../src/ml_parser/ast'; +import {DEFAULT_CONTAINER_BLOCKS, DEFAULT_INTERPOLATION_CONFIG} from '../../src/ml_parser/defaults'; import {HtmlParser} from '../../src/ml_parser/html_parser'; -import {DEFAULT_INTERPOLATION_CONFIG} from '../../src/ml_parser/interpolation_config'; describe('Message', () => { - const messageFactory = createI18nMessageFactory(DEFAULT_INTERPOLATION_CONFIG); + const messageFactory = + createI18nMessageFactory(DEFAULT_INTERPOLATION_CONFIG, DEFAULT_CONTAINER_BLOCKS); describe('messageText()', () => { it('should serialize simple text', () => { const message = messageFactory(parseHtml('abc\ndef'), '', '', ''); @@ -58,6 +59,15 @@ describe('Message', () => { .toEqual( `{VAR_SELECT_1, select, male {male of age: {VAR_SELECT, select, 10 {ten} 20 {twenty} 30 {thirty} other {other}}} female {female} other {other}}`); }); + + it('should serialize blocks', () => { + const message = messageFactory( + parseHtml('abc @if (foo) {foo} @else if (bar) {bar} @else {baz} def'), '', '', ''); + + expect(message.messageString) + .toEqual( + 'abc {$START_BLOCK_IF}foo{$CLOSE_BLOCK_IF} {$START_BLOCK_ELSE_IF}bar{$CLOSE_BLOCK_ELSE_IF} {$START_BLOCK_ELSE}baz{$CLOSE_BLOCK_ELSE} def'); + }); }); }); diff --git a/packages/compiler/test/i18n/i18n_parser_spec.ts b/packages/compiler/test/i18n/i18n_parser_spec.ts index 0e2f29fb8f07f..881b9bcbaf32b 100644 --- a/packages/compiler/test/i18n/i18n_parser_spec.ts +++ b/packages/compiler/test/i18n/i18n_parser_spec.ts @@ -9,8 +9,8 @@ import {digest, serializeNodes} from '@angular/compiler/src/i18n/digest'; import {extractMessages} from '@angular/compiler/src/i18n/extractor_merger'; import {Message} from '@angular/compiler/src/i18n/i18n_ast'; +import {DEFAULT_INTERPOLATION_CONFIG} from '@angular/compiler/src/ml_parser/defaults'; import {HtmlParser} from '@angular/compiler/src/ml_parser/html_parser'; -import {DEFAULT_INTERPOLATION_CONFIG} from '@angular/compiler/src/ml_parser/interpolation_config'; describe('I18nParser', () => { describe('elements', () => { diff --git a/packages/compiler/test/i18n/integration_common.ts b/packages/compiler/test/i18n/integration_common.ts index 53073f1162c28..4411ebb6ca261 100644 --- a/packages/compiler/test/i18n/integration_common.ts +++ b/packages/compiler/test/i18n/integration_common.ts @@ -9,8 +9,8 @@ import {NgLocalization} from '@angular/common'; import {Serializer} from '@angular/compiler/src/i18n'; import {MessageBundle} from '@angular/compiler/src/i18n/message_bundle'; +import {DEFAULT_INTERPOLATION_CONFIG} from '@angular/compiler/src/ml_parser/defaults'; import {HtmlParser} from '@angular/compiler/src/ml_parser/html_parser'; -import {DEFAULT_INTERPOLATION_CONFIG} from '@angular/compiler/src/ml_parser/interpolation_config'; import {ResourceLoader} from '@angular/compiler/src/resource_loader'; import {Component, DebugElement, TRANSLATIONS, TRANSLATIONS_FORMAT} from '@angular/core'; import {ComponentFixture, TestBed} from '@angular/core/testing'; diff --git a/packages/compiler/test/i18n/message_bundle_spec.ts b/packages/compiler/test/i18n/message_bundle_spec.ts index ddaa1b63b8386..926cf340ec7ff 100644 --- a/packages/compiler/test/i18n/message_bundle_spec.ts +++ b/packages/compiler/test/i18n/message_bundle_spec.ts @@ -10,8 +10,8 @@ import {serializeNodes} from '../../src/i18n/digest'; import * as i18n from '../../src/i18n/i18n_ast'; import {MessageBundle} from '../../src/i18n/message_bundle'; import {Serializer} from '../../src/i18n/serializers/serializer'; +import {DEFAULT_INTERPOLATION_CONFIG} from '../../src/ml_parser/defaults'; import {HtmlParser} from '../../src/ml_parser/html_parser'; -import {DEFAULT_INTERPOLATION_CONFIG} from '../../src/ml_parser/interpolation_config'; describe('MessageBundle', () => { describe('Messages', () => { diff --git a/packages/compiler/test/i18n/serializers/placeholder_spec.ts b/packages/compiler/test/i18n/serializers/placeholder_spec.ts index 5ca889f613c24..de40a0dcc414c 100644 --- a/packages/compiler/test/i18n/serializers/placeholder_spec.ts +++ b/packages/compiler/test/i18n/serializers/placeholder_spec.ts @@ -86,4 +86,16 @@ describe('PlaceholderRegistry', () => { expect(reg.getPlaceholderName('name2', 'content')).toEqual('NAME2'); }); }); + + describe('block placeholders', () => { + it('should generate placeholders for a plain block', () => { + expect(reg.getStartBlockPlaceholderName('if', [])).toBe('START_BLOCK_IF'); + expect(reg.getCloseBlockPlaceholderName('if')).toBe('CLOSE_BLOCK_IF'); + }); + + it('should generate placeholders for a block with spaces in its name', () => { + expect(reg.getStartBlockPlaceholderName('else if', [])).toBe('START_BLOCK_ELSE_IF'); + expect(reg.getCloseBlockPlaceholderName('else if')).toBe('CLOSE_BLOCK_ELSE_IF'); + }); + }); }); diff --git a/packages/compiler/test/i18n/serializers/xliff2_spec.ts b/packages/compiler/test/i18n/serializers/xliff2_spec.ts index f89a416d50385..edd1d54f6a761 100644 --- a/packages/compiler/test/i18n/serializers/xliff2_spec.ts +++ b/packages/compiler/test/i18n/serializers/xliff2_spec.ts @@ -11,8 +11,8 @@ import {escapeRegExp} from '@angular/compiler/src/util'; import {serializeNodes} from '../../../src/i18n/digest'; import {MessageBundle} from '../../../src/i18n/message_bundle'; import {Xliff2} from '../../../src/i18n/serializers/xliff2'; +import {DEFAULT_INTERPOLATION_CONFIG} from '../../../src/ml_parser/defaults'; import {HtmlParser} from '../../../src/ml_parser/html_parser'; -import {DEFAULT_INTERPOLATION_CONFIG} from '../../../src/ml_parser/interpolation_config'; const HTML = `

not translatable

@@ -26,6 +26,7 @@ const HTML = `

Test: { count, plural, =0 { { sex, select, other {

deeply nested

}} } =other {a lot}}

multi lines

+

translatable element @if (foo) {with} @else if (bar) {blocks}

`; const WRITE_XLIFF = ` @@ -125,6 +126,14 @@ const WRITE_XLIFF = ` lines + + + file.ts:13 + + + translatable element with blocks + + `; @@ -238,6 +247,15 @@ lines lignes + + + file.ts:13 + + + translatable element with blocks + élément traduisible avec des blocs + + Please check the translation for 'namespace'. On also can use 'espace de nom',but I think most technical manuals use the English term. @@ -260,26 +278,25 @@ lignes `; -(function() { -const serializer = new Xliff2(); +describe('XLIFF 2.0 serializer', () => { + const serializer = new Xliff2(); -function toXliff(html: string, locale: string|null = null): string { - const catalog = new MessageBundle(new HtmlParser, [], {}, locale); - catalog.updateFromTemplate(html, 'file.ts', DEFAULT_INTERPOLATION_CONFIG); - return catalog.write(serializer); -} + function toXliff(html: string, locale: string|null = null): string { + const catalog = new MessageBundle(new HtmlParser, [], {}, locale); + catalog.updateFromTemplate(html, 'file.ts', DEFAULT_INTERPOLATION_CONFIG); + return catalog.write(serializer); + } -function loadAsMap(xliff: string): {[id: string]: string} { - const {i18nNodesByMsgId} = serializer.load(xliff, 'url'); + function loadAsMap(xliff: string): {[id: string]: string} { + const {i18nNodesByMsgId} = serializer.load(xliff, 'url'); - const msgMap: {[id: string]: string} = {}; - Object.keys(i18nNodesByMsgId) - .forEach(id => msgMap[id] = serializeNodes(i18nNodesByMsgId[id]).join('')); + const msgMap: {[id: string]: string} = {}; + Object.keys(i18nNodesByMsgId) + .forEach(id => msgMap[id] = serializeNodes(i18nNodesByMsgId[id]).join('')); - return msgMap; -} + return msgMap; + } -describe('XLIFF 2.0 serializer', () => { describe('write', () => { it('should write a valid xliff 2.0 file', () => { expect(toXliff(HTML)).toEqual(WRITE_XLIFF); @@ -309,6 +326,8 @@ describe('XLIFF 2.0 serializer', () => { ' name="START_PARAGRAPH"/>, profondément imbriqué, ]}}, ]}, =other {[beaucoup]}}', '2340165783990709777': `multi lignes`, + '6618832065070552029': + 'élément traduisible avec des blocs', 'mrk-test': 'Vous pouvez utiliser votre propre namespace.', 'mrk-test2': 'Vous pouvez utiliser votre propre namespace.' }); @@ -439,4 +458,3 @@ lignes`, }); }); }); -})(); diff --git a/packages/compiler/test/i18n/serializers/xliff_spec.ts b/packages/compiler/test/i18n/serializers/xliff_spec.ts index 9acf397aa8686..dbc4c470a658d 100644 --- a/packages/compiler/test/i18n/serializers/xliff_spec.ts +++ b/packages/compiler/test/i18n/serializers/xliff_spec.ts @@ -11,8 +11,8 @@ import {escapeRegExp} from '@angular/compiler/src/util'; import {serializeNodes} from '../../../src/i18n/digest'; import {MessageBundle} from '../../../src/i18n/message_bundle'; import {Xliff} from '../../../src/i18n/serializers/xliff'; +import {DEFAULT_INTERPOLATION_CONFIG} from '../../../src/ml_parser/defaults'; import {HtmlParser} from '../../../src/ml_parser/html_parser'; -import {DEFAULT_INTERPOLATION_CONFIG} from '../../../src/ml_parser/interpolation_config'; const HTML = `

not translatable

@@ -27,6 +27,7 @@ const HTML = `

Test: { count, plural, =0 { { sex, select, other {

deeply nested

}} } =other {a lot}}

multi lines

+

translatable element @if (foo) {with} @else if (bar) {blocks}

`; const WRITE_XLIFF = ` @@ -120,6 +121,13 @@ lines 12 + + translatable element with blocks + + file.ts + 14 + + @@ -221,6 +229,14 @@ lignes 12 + + translatable element with blocks + élément traduisible avec des blocs + + file.ts + 14 + + First sentence. @@ -240,26 +256,25 @@ lignes `; -(function() { -const serializer = new Xliff(); +describe('XLIFF serializer', () => { + const serializer = new Xliff(); -function toXliff(html: string, locale: string|null = null): string { - const catalog = new MessageBundle(new HtmlParser, [], {}, locale); - catalog.updateFromTemplate(html, 'file.ts', DEFAULT_INTERPOLATION_CONFIG); - return catalog.write(serializer); -} + function toXliff(html: string, locale: string|null = null): string { + const catalog = new MessageBundle(new HtmlParser, [], {}, locale); + catalog.updateFromTemplate(html, 'file.ts', DEFAULT_INTERPOLATION_CONFIG); + return catalog.write(serializer); + } -function loadAsMap(xliff: string): {[id: string]: string} { - const {i18nNodesByMsgId} = serializer.load(xliff, 'url'); + function loadAsMap(xliff: string): {[id: string]: string} { + const {i18nNodesByMsgId} = serializer.load(xliff, 'url'); - const msgMap: {[id: string]: string} = {}; - Object.keys(i18nNodesByMsgId) - .forEach(id => msgMap[id] = serializeNodes(i18nNodesByMsgId[id]).join('')); + const msgMap: {[id: string]: string} = {}; + Object.keys(i18nNodesByMsgId) + .forEach(id => msgMap[id] = serializeNodes(i18nNodesByMsgId[id]).join('')); - return msgMap; -} + return msgMap; + } -describe('XLIFF serializer', () => { describe('write', () => { it('should write a valid xliff file', () => { expect(toXliff(HTML)).toEqual(WRITE_XLIFF); @@ -291,6 +306,8 @@ describe('XLIFF serializer', () => { ' name="START_PARAGRAPH"/>, profondément imbriqué, ]}}, ]}, =other {[beaucoup]}}', 'fcfa109b0e152d4c217dbc02530be0bcb8123ad1': `multi lignes`, + '3e17847a6823c7777ca57c7338167badca0f4d19': + 'élément traduisible avec des blocs', 'mrk-test': 'Translated first sentence.', 'mrk-test2': 'Translated first sentence.' }); @@ -426,4 +443,3 @@ lignes`, }); }); }); -})(); diff --git a/packages/compiler/test/i18n/serializers/xmb_spec.ts b/packages/compiler/test/i18n/serializers/xmb_spec.ts index 384a82d99c993..c9c580ab9112a 100644 --- a/packages/compiler/test/i18n/serializers/xmb_spec.ts +++ b/packages/compiler/test/i18n/serializers/xmb_spec.ts @@ -8,8 +8,8 @@ import {MessageBundle} from '@angular/compiler/src/i18n/message_bundle'; import {Xmb} from '@angular/compiler/src/i18n/serializers/xmb'; +import {DEFAULT_INTERPOLATION_CONFIG} from '@angular/compiler/src/ml_parser/defaults'; import {HtmlParser} from '@angular/compiler/src/ml_parser/html_parser'; -import {DEFAULT_INTERPOLATION_CONFIG} from '@angular/compiler/src/ml_parser/interpolation_config'; describe('XMB serializer', () => { const HTML = ` @@ -22,7 +22,8 @@ describe('XMB serializer', () => {

{ count, plural, =0 { { sex, select, other {

deeply nested

}} }}

Test: { count, plural, =0 { { sex, select, other {

deeply nested

}} } =other {a lot}}

multi -lines

`; +lines

+

translatable element @if (foo) {with} @else if (bar) {blocks}

`; const XMB = ` `; file.ts:9{VAR_PLURAL, plural, =0 {{VAR_SELECT, select, other {<p><p>deeply nested</p></p>} } } =other {a lot} } file.ts:10,11multi lines + file.ts:12translatable element @if@ifwith}} @else if@else ifblocks}} `; diff --git a/packages/compiler/test/render3/view/util.ts b/packages/compiler/test/render3/view/util.ts index 1de9ea2e03f56..afd5e95d1ae1e 100644 --- a/packages/compiler/test/render3/view/util.ts +++ b/packages/compiler/test/render3/view/util.ts @@ -10,9 +10,9 @@ import * as e from '../../../src/expression_parser/ast'; import {Lexer} from '../../../src/expression_parser/lexer'; import {Parser} from '../../../src/expression_parser/parser'; import * as html from '../../../src/ml_parser/ast'; +import {DEFAULT_INTERPOLATION_CONFIG, InterpolationConfig} from '../../../src/ml_parser/defaults'; import {HtmlParser} from '../../../src/ml_parser/html_parser'; import {WhitespaceVisitor} from '../../../src/ml_parser/html_whitespaces'; -import {DEFAULT_INTERPOLATION_CONFIG, InterpolationConfig} from '../../../src/ml_parser/interpolation_config'; import {ParseTreeResult} from '../../../src/ml_parser/parser'; import * as a from '../../../src/render3/r3_ast'; import {htmlAstToRender3Ast, Render3ParseResult} from '../../../src/render3/r3_template_transform'; From 406049b95e5234f17a7a18839ac848640f53fdde Mon Sep 17 00:00:00 2001 From: Kristiyan Kostadinov Date: Fri, 17 Nov 2023 09:13:40 +0100 Subject: [PATCH 11/19] fix(compiler): generate i18n instructions for blocks (#52958) Adds support for generating i18n instructions inside of blocks. Fixes #52540. Fixes #52767. PR Close #52958 --- .../blocks/GOLDEN_PARTIAL.js | 208 ++++++++++++++++++ .../blocks/TEST_CASES.json | 89 ++++++++ .../blocks/conditional.ts | 23 ++ .../blocks/conditional_template.js | 97 ++++++++ .../r3_view_compiler_i18n/blocks/defer.ts | 21 ++ .../blocks/defer_template.js | 101 +++++++++ .../r3_view_compiler_i18n/blocks/for.ts | 17 ++ .../blocks/for_template.js | 68 ++++++ .../r3_view_compiler_i18n/blocks/switch.ts | 17 ++ .../blocks/switch_template.js | 84 +++++++ packages/compiler/src/render3/r3_ast.ts | 18 +- .../compiler/src/render3/r3_control_flow.ts | 12 +- .../src/render3/r3_deferred_blocks.ts | 8 +- .../compiler/src/render3/view/i18n/context.ts | 13 ++ .../compiler/src/render3/view/template.ts | 45 ++-- packages/core/test/acceptance/i18n_spec.ts | 164 +++++++++++++- 16 files changed, 947 insertions(+), 38 deletions(-) create mode 100644 packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_i18n/blocks/GOLDEN_PARTIAL.js create mode 100644 packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_i18n/blocks/TEST_CASES.json create mode 100644 packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_i18n/blocks/conditional.ts create mode 100644 packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_i18n/blocks/conditional_template.js create mode 100644 packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_i18n/blocks/defer.ts create mode 100644 packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_i18n/blocks/defer_template.js create mode 100644 packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_i18n/blocks/for.ts create mode 100644 packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_i18n/blocks/for_template.js create mode 100644 packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_i18n/blocks/switch.ts create mode 100644 packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_i18n/blocks/switch_template.js diff --git a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_i18n/blocks/GOLDEN_PARTIAL.js b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_i18n/blocks/GOLDEN_PARTIAL.js new file mode 100644 index 0000000000000..5225655ba735e --- /dev/null +++ b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_i18n/blocks/GOLDEN_PARTIAL.js @@ -0,0 +1,208 @@ +/**************************************************************************************************** + * PARTIAL FILE: conditional.js + ****************************************************************************************************/ +import { Component } from '@angular/core'; +import * as i0 from "@angular/core"; +export class MyApp { + constructor() { + this.count = 0; + } +} +MyApp.ɵfac = i0.ɵɵngDeclareFactory({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyApp, deps: [], target: i0.ɵɵFactoryTarget.Component }); +MyApp.ɵcmp = i0.ɵɵngDeclareComponent({ minVersion: "17.0.0", version: "0.0.0-PLACEHOLDER", type: MyApp, selector: "ng-component", ngImport: i0, template: ` +
+ Content: + @if (count === 0) { + beforezeroafter + } @else if (count === 1) { + before
one
after + } @else { + beforeafter + }! + + @if (count === 7) { + beforesevenafter + } +
+ `, isInline: true }); +i0.ɵɵngDeclareClassMetadata({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyApp, decorators: [{ + type: Component, + args: [{ + template: ` +
+ Content: + @if (count === 0) { + beforezeroafter + } @else if (count === 1) { + before
one
after + } @else { + beforeafter + }! + + @if (count === 7) { + beforesevenafter + } +
+ ` + }] + }] }); + +/**************************************************************************************************** + * PARTIAL FILE: conditional.d.ts + ****************************************************************************************************/ +import * as i0 from "@angular/core"; +export declare class MyApp { + count: number; + static ɵfac: i0.ɵɵFactoryDeclaration; + static ɵcmp: i0.ɵɵComponentDeclaration; +} + +/**************************************************************************************************** + * PARTIAL FILE: switch.js + ****************************************************************************************************/ +import { Component } from '@angular/core'; +import * as i0 from "@angular/core"; +export class MyApp { + constructor() { + this.count = 0; + } +} +MyApp.ɵfac = i0.ɵɵngDeclareFactory({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyApp, deps: [], target: i0.ɵɵFactoryTarget.Component }); +MyApp.ɵcmp = i0.ɵɵngDeclareComponent({ minVersion: "17.0.0", version: "0.0.0-PLACEHOLDER", type: MyApp, selector: "ng-component", ngImport: i0, template: ` +
+ Content: + @switch (count) { + @case (0) {beforezeroafter} + @case (1) {before
one
after} + @default {beforeafter} + } +
+ `, isInline: true }); +i0.ɵɵngDeclareClassMetadata({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyApp, decorators: [{ + type: Component, + args: [{ + template: ` +
+ Content: + @switch (count) { + @case (0) {beforezeroafter} + @case (1) {before
one
after} + @default {beforeafter} + } +
+ ` + }] + }] }); + +/**************************************************************************************************** + * PARTIAL FILE: switch.d.ts + ****************************************************************************************************/ +import * as i0 from "@angular/core"; +export declare class MyApp { + count: number; + static ɵfac: i0.ɵɵFactoryDeclaration; + static ɵcmp: i0.ɵɵComponentDeclaration; +} + +/**************************************************************************************************** + * PARTIAL FILE: for.js + ****************************************************************************************************/ +import { Component } from '@angular/core'; +import * as i0 from "@angular/core"; +export class MyApp { + constructor() { + this.items = [1, 2, 3]; + } +} +MyApp.ɵfac = i0.ɵɵngDeclareFactory({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyApp, deps: [], target: i0.ɵɵFactoryTarget.Component }); +MyApp.ɵcmp = i0.ɵɵngDeclareComponent({ minVersion: "17.0.0", version: "0.0.0-PLACEHOLDER", type: MyApp, selector: "ng-component", ngImport: i0, template: ` +
+ Content: + @for (item of items; track item) { + beforemiddleafter + } @empty { + before
empty
after + }! +
+ `, isInline: true }); +i0.ɵɵngDeclareClassMetadata({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyApp, decorators: [{ + type: Component, + args: [{ + template: ` +
+ Content: + @for (item of items; track item) { + beforemiddleafter + } @empty { + before
empty
after + }! +
+ ` + }] + }] }); + +/**************************************************************************************************** + * PARTIAL FILE: for.d.ts + ****************************************************************************************************/ +import * as i0 from "@angular/core"; +export declare class MyApp { + items: number[]; + static ɵfac: i0.ɵɵFactoryDeclaration; + static ɵcmp: i0.ɵɵComponentDeclaration; +} + +/**************************************************************************************************** + * PARTIAL FILE: defer.js + ****************************************************************************************************/ +import { Component } from '@angular/core'; +import * as i0 from "@angular/core"; +export class MyApp { + constructor() { + this.isLoaded = false; + } +} +MyApp.ɵfac = i0.ɵɵngDeclareFactory({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyApp, deps: [], target: i0.ɵɵFactoryTarget.Component }); +MyApp.ɵcmp = i0.ɵɵngDeclareComponent({ minVersion: "17.0.0", version: "0.0.0-PLACEHOLDER", type: MyApp, selector: "ng-component", ngImport: i0, template: ` +
+ Content: + @defer (when isLoaded) { + beforemiddleafter + } @placeholder { + before
placeholder
after + } @loading { + beforeafter + } @error { + before

error

after + } +
+ `, isInline: true }); +i0.ɵɵngDeclareClassMetadata({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyApp, decorators: [{ + type: Component, + args: [{ + template: ` +
+ Content: + @defer (when isLoaded) { + beforemiddleafter + } @placeholder { + before
placeholder
after + } @loading { + beforeafter + } @error { + before

error

after + } +
+ ` + }] + }] }); + +/**************************************************************************************************** + * PARTIAL FILE: defer.d.ts + ****************************************************************************************************/ +import * as i0 from "@angular/core"; +export declare class MyApp { + isLoaded: boolean; + static ɵfac: i0.ɵɵFactoryDeclaration; + static ɵcmp: i0.ɵɵComponentDeclaration; +} + diff --git a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_i18n/blocks/TEST_CASES.json b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_i18n/blocks/TEST_CASES.json new file mode 100644 index 0000000000000..305e16408118e --- /dev/null +++ b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_i18n/blocks/TEST_CASES.json @@ -0,0 +1,89 @@ +{ + "$schema": "../../test_case_schema.json", + "cases": [ + { + "description": "should support @if blocks", + "skipForTemplatePipeline": true, + "inputFiles": [ + "conditional.ts" + ], + "expectations": [ + { + "files": [ + { + "generated": "conditional.js", + "expected": "conditional_template.js" + } + ], + "extraChecks": [ + "verifyPlaceholdersIntegrity", + "verifyUniqueConsts" + ] + } + ] + }, + { + "description": "should support @switch blocks", + "skipForTemplatePipeline": true, + "inputFiles": [ + "switch.ts" + ], + "expectations": [ + { + "files": [ + { + "generated": "switch.js", + "expected": "switch_template.js" + } + ], + "extraChecks": [ + "verifyPlaceholdersIntegrity", + "verifyUniqueConsts" + ] + } + ] + }, + { + "description": "should support @for blocks", + "skipForTemplatePipeline": true, + "inputFiles": [ + "for.ts" + ], + "expectations": [ + { + "files": [ + { + "generated": "for.js", + "expected": "for_template.js" + } + ], + "extraChecks": [ + "verifyPlaceholdersIntegrity", + "verifyUniqueConsts" + ] + } + ] + }, + { + "description": "should support @defer blocks", + "skipForTemplatePipeline": true, + "inputFiles": [ + "defer.ts" + ], + "expectations": [ + { + "files": [ + { + "generated": "defer.js", + "expected": "defer_template.js" + } + ], + "extraChecks": [ + "verifyPlaceholdersIntegrity", + "verifyUniqueConsts" + ] + } + ] + } + ] +} diff --git a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_i18n/blocks/conditional.ts b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_i18n/blocks/conditional.ts new file mode 100644 index 0000000000000..5ac5bd3bc1996 --- /dev/null +++ b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_i18n/blocks/conditional.ts @@ -0,0 +1,23 @@ +import {Component} from '@angular/core'; + +@Component({ + template: ` +
+ Content: + @if (count === 0) { + beforezeroafter + } @else if (count === 1) { + before
one
after + } @else { + beforeafter + }! + + @if (count === 7) { + beforesevenafter + } +
+ ` +}) +export class MyApp { + count = 0; +} diff --git a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_i18n/blocks/conditional_template.js b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_i18n/blocks/conditional_template.js new file mode 100644 index 0000000000000..a159da1c7e9ea --- /dev/null +++ b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_i18n/blocks/conditional_template.js @@ -0,0 +1,97 @@ +function MyApp_Conditional_2_Template(rf, ctx) { + if (rf & 1) { + $r3$.ɵɵi18nStart(0, 0, 1); + $r3$.ɵɵelement(1, "span"); + $r3$.ɵɵi18nEnd(); + } +} + +function MyApp_Conditional_3_Template(rf, ctx) { + if (rf & 1) { + $r3$.ɵɵi18nStart(0, 0, 2); + $r3$.ɵɵelement(1, "div"); + $r3$.ɵɵi18nEnd(); + } +} + +function MyApp_Conditional_4_Template(rf, ctx) { + if (rf & 1) { + $r3$.ɵɵi18nStart(0, 0, 3); + $r3$.ɵɵelement(1, "button"); + $r3$.ɵɵi18nEnd(); + } +} + +function MyApp_Conditional_5_Template(rf, ctx) { + if (rf & 1) { + $r3$.ɵɵi18nStart(0, 0, 4); + $r3$.ɵɵelement(1, "span"); + $r3$.ɵɵi18nEnd(); + } +} + +… + +MyApp.ɵcmp = /*@__PURE__*/ $r3$.ɵɵdefineComponent({ + … + consts: () => { + let i18n_0; + if (typeof ngI18nClosureMode !== "undefined" && ngI18nClosureMode) { + /** + * @suppress {msgDescriptions} + */ + const MSG_EXTERNAL_4037030454066434177$$CONDITIONAL_TS__1 = goog.getMsg(" Content: {$startBlockIf} before{$startTagSpan}zero{$closeTagSpan}after {$closeBlockIf}{$startBlockElseIf} before{$startTagDiv}one{$closeTagDiv}after {$closeBlockElseIf}{$startBlockElse} before{$startTagButton}otherwise{$closeTagButton}after {$closeBlockElse}! {$startBlockIf_1} before{$startTagSpan}seven{$closeTagSpan}after {$closeBlockIf}", { + "closeBlockElse": "\uFFFD/*4:3\uFFFD", + "closeBlockElseIf": "\uFFFD/*3:2\uFFFD", + "closeBlockIf": "[\uFFFD/*2:1\uFFFD|\uFFFD/*5:4\uFFFD]", + "closeTagButton": "\uFFFD/#1:3\uFFFD", + "closeTagDiv": "\uFFFD/#1:2\uFFFD", + "closeTagSpan": "[\uFFFD/#1:1\uFFFD|\uFFFD/#1:4\uFFFD]", + "startBlockElse": "\uFFFD*4:3\uFFFD", + "startBlockElseIf": "\uFFFD*3:2\uFFFD", + "startBlockIf": "\uFFFD*2:1\uFFFD", + "startBlockIf_1": "\uFFFD*5:4\uFFFD", + "startTagButton": "\uFFFD#1:3\uFFFD", + "startTagDiv": "\uFFFD#1:2\uFFFD", + "startTagSpan": "[\uFFFD#1:1\uFFFD|\uFFFD#1:4\uFFFD]" + }, { + original_code: { + "closeBlockElse": "}", + "closeBlockElseIf": "}", + "closeBlockIf": "}", + "closeTagButton": "", + "closeTagDiv": "", + "closeTagSpan": "", + "startBlockElse": "@else {", + "startBlockElseIf": "@else if (count === 1) {", + "startBlockIf": "@if (count === 0) {", + "startBlockIf_1": "@if (count === 7) {", + "startTagButton": "after + } @error { + before

error

after + } + + ` +}) +export class MyApp { + isLoaded = false; +} diff --git a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_i18n/blocks/defer_template.js b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_i18n/blocks/defer_template.js new file mode 100644 index 0000000000000..6bf70b8dfa5a5 --- /dev/null +++ b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_i18n/blocks/defer_template.js @@ -0,0 +1,101 @@ +function MyApp_Defer_2_Template(rf, ctx) { + if (rf & 1) { + $r3$.ɵɵi18nStart(0, 0, 1); + $r3$.ɵɵelement(1, "span"); + $r3$.ɵɵi18nEnd(); + } +} + +function MyApp_DeferLoading_3_Template(rf, ctx) { + if (rf & 1) { + $r3$.ɵɵi18nStart(0, 0, 2); + $r3$.ɵɵelement(1, "button"); + $r3$.ɵɵi18nEnd(); + } +} + +function MyApp_DeferPlaceholder_4_Template(rf, ctx) { + if (rf & 1) { + $r3$.ɵɵi18nStart(0, 0, 3); + $r3$.ɵɵelement(1, "div"); + $r3$.ɵɵi18nEnd(); + } +} + +function MyApp_DeferError_5_Template(rf, ctx) { + if (rf & 1) { + $r3$.ɵɵi18nStart(0, 0, 4); + $r3$.ɵɵelement(1, "h1"); + $r3$.ɵɵi18nEnd(); + } +} + +… + +MyApp.ɵcmp = /*@__PURE__*/ $r3$.ɵɵdefineComponent({ + … + consts: () => { + let i18n_0; + if (typeof ngI18nClosureMode !== "undefined" && ngI18nClosureMode) { + /** + * @suppress {msgDescriptions} + */ + const MSG_EXTERNAL_7863738818382242773$$DEFER_TS__1 = goog.getMsg(" Content: {$startBlockDefer} before{$startTagSpan}middle{$closeTagSpan}after {$closeBlockDefer}{$startBlockPlaceholder} before{$startTagDiv}placeholder{$closeTagDiv}after {$closeBlockPlaceholder}{$startBlockLoading} before{$startTagButton}loading{$closeTagButton}after {$closeBlockLoading}{$startBlockError} before{$startHeadingLevel1}error{$closeHeadingLevel1}after {$closeBlockError}", { + "closeBlockDefer": "\uFFFD/*2:1\uFFFD", + "closeBlockError": "\uFFFD/*5:4\uFFFD", + "closeBlockLoading": "\uFFFD/*3:2\uFFFD", + "closeBlockPlaceholder": "\uFFFD/*4:3\uFFFD", + "closeHeadingLevel1": "\uFFFD/#1:4\uFFFD", + "closeTagButton": "\uFFFD/#1:2\uFFFD", + "closeTagDiv": "\uFFFD/#1:3\uFFFD", + "closeTagSpan": "\uFFFD/#1:1\uFFFD", + "startBlockDefer": "\uFFFD*2:1\uFFFD", + "startBlockError": "\uFFFD*5:4\uFFFD", + "startBlockLoading": "\uFFFD*3:2\uFFFD", + "startBlockPlaceholder": "\uFFFD*4:3\uFFFD", + "startHeadingLevel1": "\uFFFD#1:4\uFFFD", + "startTagButton": "\uFFFD#1:2\uFFFD", + "startTagDiv": "\uFFFD#1:3\uFFFD", + "startTagSpan": "\uFFFD#1:1\uFFFD" + }, { + original_code: { + "closeBlockDefer": "}", + "closeBlockError": "}", + "closeBlockLoading": "}", + "closeBlockPlaceholder": "}", + "closeHeadingLevel1": "", + "closeTagButton": "", + "closeTagDiv": "", + "closeTagSpan": "", + "startBlockDefer": "@defer (when isLoaded) {", + "startBlockError": "@error {", + "startBlockLoading": "@loading {", + "startBlockPlaceholder": "@placeholder {", + "startHeadingLevel1": "

", + "startTagButton": "after} + } + + ` +}) +export class MyApp { + count = 0; +} diff --git a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_i18n/blocks/switch_template.js b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_i18n/blocks/switch_template.js new file mode 100644 index 0000000000000..6581d336c8bfb --- /dev/null +++ b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_i18n/blocks/switch_template.js @@ -0,0 +1,84 @@ +function MyApp_Case_2_Template(rf, ctx) { + if (rf & 1) { + $r3$.ɵɵi18nStart(0, 0, 1); + $r3$.ɵɵelement(1, "span"); + $r3$.ɵɵi18nEnd(); + } +} + +function MyApp_Case_3_Template(rf, ctx) { + if (rf & 1) { + $r3$.ɵɵi18nStart(0, 0, 2); + $r3$.ɵɵelement(1, "div"); + $r3$.ɵɵi18nEnd(); + } +} + +function MyApp_Case_4_Template(rf, ctx) { + if (rf & 1) { + $r3$.ɵɵi18nStart(0, 0, 3); + $r3$.ɵɵelement(1, "button"); + $r3$.ɵɵi18nEnd(); + } +} + +… + +MyApp.ɵcmp = /*@__PURE__*/ $r3$.ɵɵdefineComponent({ + … + consts: () => { + let i18n_0; + if (typeof ngI18nClosureMode !== "undefined" && ngI18nClosureMode) { + /** + * @suppress {msgDescriptions} + */ + const MSG_EXTERNAL_532730339085995504$$SWITCH_TS__1 = goog.getMsg(" Content: {$startBlockCase}before{$startTagSpan}zero{$closeTagSpan}after{$closeBlockCase}{$startBlockCase_1}before{$startTagDiv}one{$closeTagDiv}after{$closeBlockCase}{$startBlockDefault}before{$startTagButton}otherwise{$closeTagButton}after{$closeBlockDefault}", { + "closeBlockCase": "[\uFFFD/*2:1\uFFFD|\uFFFD/*3:2\uFFFD]", + "closeBlockDefault": "\uFFFD/*4:3\uFFFD", + "closeTagButton": "\uFFFD/#1:3\uFFFD", + "closeTagDiv": "\uFFFD/#1:2\uFFFD", + "closeTagSpan": "\uFFFD/#1:1\uFFFD", + "startBlockCase": "\uFFFD*2:1\uFFFD", + "startBlockCase_1": "\uFFFD*3:2\uFFFD", + "startBlockDefault": "\uFFFD*4:3\uFFFD", + "startTagButton": "\uFFFD#1:3\uFFFD", + "startTagDiv": "\uFFFD#1:2\uFFFD", + "startTagSpan": "\uFFFD#1:1\uFFFD" + }, { + original_code: { + "closeBlockCase": "}", + "closeBlockDefault": "}", + "closeTagButton": "", + "closeTagDiv": "", + "closeTagSpan": "", + "startBlockCase": "@case (0) {", + "startBlockCase_1": "@case (1) {", + "startBlockDefault": "@default {", + "startTagButton": "after}!'); + + expect(fixture.nativeElement.innerHTML) + .toEqual( + '
Contenido: antescerodespués' + + '!
'); + + fixture.componentInstance.count = 1; + fixture.detectChanges(); + expect(fixture.nativeElement.innerHTML) + .toEqual( + '
Contenido: antes
uno
después' + + '!
'); + + fixture.componentInstance.count = 2; + fixture.detectChanges(); + expect(fixture.nativeElement.innerHTML) + .toEqual( + '
Contenido: antesdespués' + + '!
'); + }); + + it('should support switch blocks', () => { + loadTranslations({ + [computeMsgId( + 'Content: {$START_BLOCK_CASE}before{$START_TAG_SPAN}zero{$CLOSE_TAG_SPAN}after' + + '{$CLOSE_BLOCK_CASE}{$START_BLOCK_CASE_1}before{$START_TAG_DIV}one' + + '{$CLOSE_TAG_DIV}after{$CLOSE_BLOCK_CASE}{$START_BLOCK_DEFAULT}before' + + '{$START_TAG_BUTTON}otherwise{$CLOSE_TAG_BUTTON}after{$CLOSE_BLOCK_DEFAULT}', + '')]: 'Contenido: {$START_BLOCK_CASE}antes{$START_TAG_SPAN}cero{$CLOSE_TAG_SPAN}después' + + '{$CLOSE_BLOCK_CASE}{$START_BLOCK_CASE_1}antes{$START_TAG_DIV}uno' + + '{$CLOSE_TAG_DIV}después{$CLOSE_BLOCK_CASE}{$START_BLOCK_DEFAULT}antes' + + '{$START_TAG_BUTTON}si no{$CLOSE_TAG_BUTTON}después{$CLOSE_BLOCK_DEFAULT}' + }); + + const fixture = initWithTemplate( + AppComp, + '
Content: @switch (count) {' + + '@case (0) {beforezeroafter}' + + '@case (1) {before
one
after}' + + '@default {beforeafter}' + + '}
'); + + expect(fixture.nativeElement.innerHTML) + .toEqual( + '
Contenido: antescerodespués' + + '
'); + + fixture.componentInstance.count = 1; + fixture.detectChanges(); + expect(fixture.nativeElement.innerHTML) + .toEqual( + '
Contenido: antes
uno
después' + + '
'); + + fixture.componentInstance.count = 2; + fixture.detectChanges(); + expect(fixture.nativeElement.innerHTML) + .toEqual( + '
Contenido: antesdespués' + + '
'); + }); + + it('should support for loop blocks', () => { + loadTranslations({ + [computeMsgId( + 'Content: {$START_BLOCK_FOR}before{$START_TAG_SPAN}' + + 'middle{$CLOSE_TAG_SPAN}after{$CLOSE_BLOCK_FOR}{$START_BLOCK_EMPTY}' + + 'before{$START_TAG_DIV}empty{$CLOSE_TAG_DIV}after{$CLOSE_BLOCK_EMPTY}!')]: + 'Contenido: {$START_BLOCK_FOR}antes{$START_TAG_SPAN}' + + 'medio{$CLOSE_TAG_SPAN}después{$CLOSE_BLOCK_FOR}{$START_BLOCK_EMPTY}' + + 'antes{$START_TAG_DIV}vacío{$CLOSE_TAG_DIV}después{$CLOSE_BLOCK_EMPTY}!' + }); + + const fixture = initWithTemplate( + AppComp, + '
Content: @for (item of items; track item) {beforemiddleafter}' + + '@empty {before
empty
after}!
'); + + expect(fixture.nativeElement.innerHTML) + .toEqual( + '
Contenido: antesmedio' + + 'despuésantesmediodespuésantesmedio' + + 'después!
'); + + fixture.componentInstance.items = []; + fixture.detectChanges(); + + expect(fixture.nativeElement.innerHTML) + .toEqual( + '
Contenido: antes
' + + 'vacío
después!
'); + }); + + it('should support deferred blocks', async () => { + loadTranslations({ + [computeMsgId( + 'Content: {$START_BLOCK_DEFER}before{$START_TAG_SPAN}middle' + + '{$CLOSE_TAG_SPAN}after{$CLOSE_BLOCK_DEFER}{$START_BLOCK_PLACEHOLDER}before' + + '{$START_TAG_DIV}placeholder{$CLOSE_TAG_DIV}after{$CLOSE_BLOCK_PLACEHOLDER}!', + '')]: 'Contenido: {$START_BLOCK_DEFER}before{$START_TAG_SPAN}medio' + + '{$CLOSE_TAG_SPAN}after{$CLOSE_BLOCK_DEFER}{$START_BLOCK_PLACEHOLDER}before' + + '{$START_TAG_DIV}marcador de posición{$CLOSE_TAG_DIV}after{$CLOSE_BLOCK_PLACEHOLDER}!' + }); + + @Component({ + selector: 'defer-comp', + standalone: true, + template: '
Content: @defer (when isLoaded) {beforemiddleafter} ' + + '@placeholder {before
placeholder
after}!
' + }) + class DeferComp { + isLoaded = false; + } + + TestBed.configureTestingModule({ + imports: [DeferComp], + deferBlockBehavior: DeferBlockBehavior.Manual, + teardown: {destroyAfterEach: true}, + }); + + const fixture = TestBed.createComponent(DeferComp); + fixture.detectChanges(); + + expect(fixture.nativeElement.innerHTML) + .toEqual( + '
Contenido: ' + + '!before
marcador de posición
after
'); + + const deferBlock = (await fixture.getDeferBlocks())[0]; + fixture.componentInstance.isLoaded = true; + fixture.detectChanges(); + await deferBlock.render(DeferBlockState.Complete); + + expect(fixture.nativeElement.innerHTML) + .toEqual( + '
Contenido: ' + + '!beforemedioafter
'); + }); + describe('ng-container and ng-template support', () => { it('should support ng-container', () => { loadTranslations({[computeMsgId('text')]: 'texte'}); @@ -3061,6 +3218,7 @@ class AppComp { description = `Web Framework`; visible = true; count = 0; + items = [1, 2, 3]; } @Component({ From e090b48bf8534761d46523be57a7889a325bcdec Mon Sep 17 00:00:00 2001 From: Jessica Janiuk Date: Fri, 17 Nov 2023 16:33:05 -0500 Subject: [PATCH 12/19] fix(migrations): tweaks to formatting in control flow migration (#53058) This addresses a few minor formatting issues with the control flow migration. fixes: #53017 PR Close #53058 --- .../control-flow-migration/cases.ts | 9 +- .../control-flow-migration/fors.ts | 9 +- .../ng-generate/control-flow-migration/ifs.ts | 9 +- .../control-flow-migration/migration.ts | 4 +- .../control-flow-migration/switches.ts | 9 +- .../control-flow-migration/util.ts | 33 +++++-- .../test/control_flow_migration_spec.ts | 96 ++++++++++++++++--- 7 files changed, 136 insertions(+), 33 deletions(-) diff --git a/packages/core/schematics/ng-generate/control-flow-migration/cases.ts b/packages/core/schematics/ng-generate/control-flow-migration/cases.ts index dc50096034561..7123967b5309a 100644 --- a/packages/core/schematics/ng-generate/control-flow-migration/cases.ts +++ b/packages/core/schematics/ng-generate/control-flow-migration/cases.ts @@ -29,11 +29,12 @@ const cases = [ * Replaces structural directive ngSwitch instances with new switch. * Returns null if the migration failed (e.g. there was a syntax error). */ -export function migrateCase(template: string): {migrated: string, errors: MigrateError[]} { +export function migrateCase(template: string): + {migrated: string, errors: MigrateError[], changed: boolean} { let errors: MigrateError[] = []; let parsed = parseTemplate(template); if (parsed === null) { - return {migrated: template, errors}; + return {migrated: template, errors, changed: false}; } let result = template; @@ -73,7 +74,9 @@ export function migrateCase(template: string): {migrated: string, errors: Migrat nestLevel = el.nestCount; } - return {migrated: result, errors}; + const changed = visitor.elements.length > 0; + + return {migrated: result, errors, changed}; } function migrateNgSwitchCase(etm: ElementToMigrate, tmpl: string, offset: number): Result { diff --git a/packages/core/schematics/ng-generate/control-flow-migration/fors.ts b/packages/core/schematics/ng-generate/control-flow-migration/fors.ts index 0d7082a539168..5882cf4147fae 100644 --- a/packages/core/schematics/ng-generate/control-flow-migration/fors.ts +++ b/packages/core/schematics/ng-generate/control-flow-migration/fors.ts @@ -32,11 +32,12 @@ export const stringPairs = new Map([ * Replaces structural directive ngFor instances with new for. * Returns null if the migration failed (e.g. there was a syntax error). */ -export function migrateFor(template: string): {migrated: string, errors: MigrateError[]} { +export function migrateFor(template: string): + {migrated: string, errors: MigrateError[], changed: boolean} { let errors: MigrateError[] = []; let parsed = parseTemplate(template); if (parsed === null) { - return {migrated: template, errors}; + return {migrated: template, errors, changed: false}; } let result = template; @@ -68,7 +69,9 @@ export function migrateFor(template: string): {migrated: string, errors: Migrate nestLevel = el.nestCount; } - return {migrated: result, errors}; + const changed = visitor.elements.length > 0; + + return {migrated: result, errors, changed}; } function migrateNgFor(etm: ElementToMigrate, tmpl: string, offset: number): Result { diff --git a/packages/core/schematics/ng-generate/control-flow-migration/ifs.ts b/packages/core/schematics/ng-generate/control-flow-migration/ifs.ts index 959623a4820bf..3dd03971f36e1 100644 --- a/packages/core/schematics/ng-generate/control-flow-migration/ifs.ts +++ b/packages/core/schematics/ng-generate/control-flow-migration/ifs.ts @@ -25,11 +25,12 @@ const ifs = [ * Replaces structural directive ngif instances with new if. * Returns null if the migration failed (e.g. there was a syntax error). */ -export function migrateIf(template: string): {migrated: string, errors: MigrateError[]} { +export function migrateIf(template: string): + {migrated: string, errors: MigrateError[], changed: boolean} { let errors: MigrateError[] = []; let parsed = parseTemplate(template); if (parsed === null) { - return {migrated: template, errors}; + return {migrated: template, errors, changed: false}; } let result = template; @@ -61,7 +62,9 @@ export function migrateIf(template: string): {migrated: string, errors: MigrateE nestLevel = el.nestCount; } - return {migrated: result, errors}; + const changed = visitor.elements.length > 0; + + return {migrated: result, errors, changed}; } function migrateNgIf(etm: ElementToMigrate, tmpl: string, offset: number): Result { diff --git a/packages/core/schematics/ng-generate/control-flow-migration/migration.ts b/packages/core/schematics/ng-generate/control-flow-migration/migration.ts index 09d9f81f4d155..711c68079f698 100644 --- a/packages/core/schematics/ng-generate/control-flow-migration/migration.ts +++ b/packages/core/schematics/ng-generate/control-flow-migration/migration.ts @@ -29,7 +29,9 @@ export function migrateTemplate( const switchResult = migrateSwitch(forResult.migrated); const caseResult = migrateCase(switchResult.migrated); migrated = processNgTemplates(caseResult.migrated); - if (format) { + const changed = + ifResult.changed || forResult.changed || switchResult.changed || caseResult.changed; + if (format && changed) { migrated = formatTemplate(migrated); } file.removeCommonModule = canRemoveCommonModule(template); diff --git a/packages/core/schematics/ng-generate/control-flow-migration/switches.ts b/packages/core/schematics/ng-generate/control-flow-migration/switches.ts index 6656f501bd8a6..69fd45dbd8034 100644 --- a/packages/core/schematics/ng-generate/control-flow-migration/switches.ts +++ b/packages/core/schematics/ng-generate/control-flow-migration/switches.ts @@ -21,11 +21,12 @@ const switches = [ * Replaces structural directive ngSwitch instances with new switch. * Returns null if the migration failed (e.g. there was a syntax error). */ -export function migrateSwitch(template: string): {migrated: string, errors: MigrateError[]} { +export function migrateSwitch(template: string): + {migrated: string, errors: MigrateError[], changed: boolean} { let errors: MigrateError[] = []; let parsed = parseTemplate(template); if (parsed === null) { - return {migrated: template, errors}; + return {migrated: template, errors, changed: false}; } let result = template; @@ -59,7 +60,9 @@ export function migrateSwitch(template: string): {migrated: string, errors: Migr nestLevel = el.nestCount; } - return {migrated: result, errors}; + const changed = visitor.elements.length > 0; + + return {migrated: result, errors, changed}; } function migrateNgSwitch(etm: ElementToMigrate, tmpl: string, offset: number): Result { diff --git a/packages/core/schematics/ng-generate/control-flow-migration/util.ts b/packages/core/schematics/ng-generate/control-flow-migration/util.ts index e925b8e0752b2..d7b36dff1fff7 100644 --- a/packages/core/schematics/ng-generate/control-flow-migration/util.ts +++ b/packages/core/schematics/ng-generate/control-flow-migration/util.ts @@ -33,13 +33,30 @@ export function analyze(sourceFile: ts.SourceFile, analyzedFiles: Map ||
]*\/>)[^>]*>?/; + const openElRegex = /^\s*<([a-z0-9]+)(?![^>]*\/>)[^>]*>?/; // match closing block or else block // } | } @else @@ -431,7 +448,7 @@ export function formatTemplate(tmpl: string): string { // matches closing of an html element // - const closeElRegex = /\s*<\/([a-z\-]+)*>/; + const closeElRegex = /\s*<\/([a-z0-9\-]+)*>/; // matches closing of a self closing html element when the element is on multiple lines // [binding]="value" /> @@ -439,13 +456,13 @@ export function formatTemplate(tmpl: string): string { // matches an open and close of an html element on a single line with no breaks //
blah
- const singleLineElRegex = /^\s*<([a-z]+)(?![^>]*\/>)[^>]*>.*<\/([a-z\-]+)*>/; + const singleLineElRegex = /^\s*<([a-z0-9]+)(?![^>]*\/>)[^>]*>.*<\/([a-z0-9\-]+)*>/; const lines = tmpl.split('\n'); const formatted = []; let indent = ''; - for (let line of lines) { - if (line.trim() === '') { - // skip blank lines + for (let [index, line] of lines.entries()) { + if (line.trim() === '' && index !== 0 && index !== lines.length - 1) { + // skip blank lines except if it's the first line or last line continue; } if ((closeBlockRegex.test(line) || diff --git a/packages/core/schematics/test/control_flow_migration_spec.ts b/packages/core/schematics/test/control_flow_migration_spec.ts index c06e74cd898ef..747211eaac87a 100644 --- a/packages/core/schematics/test/control_flow_migration_spec.ts +++ b/packages/core/schematics/test/control_flow_migration_spec.ts @@ -357,7 +357,7 @@ describe('control flow migration', () => { ` } @else {`, `

Stuff

`, ` }`, - `
`, + `\n`, ].join('\n')); }); @@ -392,7 +392,7 @@ describe('control flow migration', () => { ` } @else {`, `

Stuff

`, ` }`, - ``, + `\n`, ].join('\n')); }); @@ -582,7 +582,7 @@ describe('control flow migration', () => { ` } @else {`, `

other

`, ` }`, - ``, + `\n`, ].join('\n')); }); @@ -616,7 +616,7 @@ describe('control flow migration', () => { ` } @else {`, `

other

`, ` }`, - ``, + `\n`, ].join('\n')); }); @@ -876,7 +876,7 @@ describe('control flow migration', () => { ` Empty`, ` }`, ` }`, - ``, + `\n`, ].join('\n')); }); @@ -3081,7 +3081,7 @@ describe('control flow migration', () => { ` height="2rem"`, ` width="6rem" />`, ` }`, - ``, + `\n`, ].join('\n'); expect(content).toBe(result); @@ -3122,7 +3122,7 @@ describe('control flow migration', () => { `
`, ` }`, ` }`, - ``, + `\n`, ].join('\n'); expect(actual).toBe(expected); @@ -3165,7 +3165,7 @@ describe('control flow migration', () => { ` }`, ` `, ` }`, - ``, + `\n`, ].join('\n'); expect(actual).toBe(expected); @@ -3208,7 +3208,7 @@ describe('control flow migration', () => { ` } @else {`, `

No Permissions

`, ` }`, - ``, + `\n`, ].join('\n'); expect(actual).toBe(expected); @@ -3228,8 +3228,10 @@ describe('control flow migration', () => { `); writeFile('/comp.html', [ + `
changed
`, `
`, `@if (stuff) {`, + `

Title

`, `

Stuff

`, `} @else if (things) {`, `

Things

`, @@ -3243,8 +3245,12 @@ describe('control flow migration', () => { const actual = tree.readContent('/comp.html'); const expected = [ + `@if (true) {`, + `
changed
`, + `}`, `
`, ` @if (stuff) {`, + `

Title

`, `

Stuff

`, ` } @else if (things) {`, `

Things

`, @@ -3270,6 +3276,7 @@ describe('control flow migration', () => { `); writeFile('/comp.html', [ + `
changed
`, `
`, `@if (stuff) {`, `stuff`, @@ -3277,13 +3284,16 @@ describe('control flow migration', () => { ``, `}`, - `
`, + `
\n`, ].join('\n')); await runMigration(); const actual = tree.readContent('/comp.html'); const expected = [ + `@if (true) {`, + `
changed
`, + `}`, `
`, ` @if (stuff) {`, ` stuff`, @@ -3291,7 +3301,7 @@ describe('control flow migration', () => { ` stuff`, ` }`, - `
`, + `
\n`, ].join('\n'); expect(actual).toBe(expected); @@ -3362,6 +3372,68 @@ describe('control flow migration', () => { expect(actual).toBe(expected); }); + it('should not remove common module imports post migration if other items used', async () => { + writeFile('/comp.ts', [ + `import {CommonModule} from '@angular/common';`, + `import {Component} from '@angular/core';\n`, + `@Component({`, + ` imports: [NgIf, DatePipe],`, + ` template: \`
{{ d | date }}
\``, + `})`, + `class Comp {`, + ` toggle = false;`, + `}`, + ].join('\n')); + + await runMigration(); + const actual = tree.readContent('/comp.ts'); + const expected = [ + `import {CommonModule} from '@angular/common';`, + `import {Component} from '@angular/core';\n`, + `@Component({`, + ` imports: [DatePipe],`, + ` template: \`
@if (toggle) {{{ d | date }}}
\``, + `})`, + `class Comp {`, + ` toggle = false;`, + `}`, + ].join('\n'); + + expect(actual).toBe(expected); + }); + + it('should not duplicate comments post migration if other items used', async () => { + writeFile('/comp.ts', [ + `// comment here`, + `import {NgIf, CommonModule} from '@angular/common';`, + `import {Component} from '@angular/core';\n`, + `@Component({`, + ` imports: [NgIf, DatePipe],`, + ` template: \`
{{ d | date }}
\``, + `})`, + `class Comp {`, + ` toggle = false;`, + `}`, + ].join('\n')); + + await runMigration(); + const actual = tree.readContent('/comp.ts'); + const expected = [ + `// comment here`, + `import { CommonModule } from '@angular/common';`, + `import {Component} from '@angular/core';\n`, + `@Component({`, + ` imports: [DatePipe],`, + ` template: \`
@if (toggle) {{{ d | date }}}
\``, + `})`, + `class Comp {`, + ` toggle = false;`, + `}`, + ].join('\n'); + + expect(actual).toBe(expected); + }); + it('should leave non-cf common module imports post migration', async () => { writeFile('/comp.ts', [ `import {Component} from '@angular/core';`, @@ -3438,7 +3510,7 @@ describe('control flow migration', () => { const actual = tree.readContent('/comp.ts'); const expected = [ `import {Component} from '@angular/core';`, - `import { CommonModule } from '@angular/common';\n`, + `import {CommonModule} from '@angular/common';\n`, `@Component({`, ` imports: [CommonModule],`, ` template: \`
@if (toggle) {{{ shrug | lowercase }}}
\``, From 2ecda9fc4bed5bf8265f12b8407f4669a5970127 Mon Sep 17 00:00:00 2001 From: Andrew Kushnir Date: Mon, 20 Nov 2023 19:46:06 +0000 Subject: [PATCH 13/19] docs: release notes for the v17.0.4 release --- CHANGELOG.md | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index f0fd98cc2e02a..0087fdcb65796 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,36 @@ + +# 17.0.4 (2023-11-20) +### common +| Commit | Type | Description | +| -- | -- | -- | +| [7f1c55755d](https://github.com/angular/angular/commit/7f1c55755d94444aa2c07fc62c276bb158e69f24) | fix | remove `load` on image once it fails to load ([#52990](https://github.com/angular/angular/pull/52990)) | +| [fafcb0d23f](https://github.com/angular/angular/commit/fafcb0d23f1f687a2fe5c8349b916586ffadc375) | fix | scan images once page is loaded ([#52991](https://github.com/angular/angular/pull/52991)) | +### compiler +| Commit | Type | Description | +| -- | -- | -- | +| [98376f2c09](https://github.com/angular/angular/commit/98376f2c09e9c28d1473123a2a1f4fb1c9d1cb1e) | fix | changed after checked error in for loops ([#52935](https://github.com/angular/angular/pull/52935)) | +| [291deac663](https://github.com/angular/angular/commit/291deac6636a6f99a98dd0c9096ebe3b0547bb9e) | fix | generate i18n instructions for blocks ([#52958](https://github.com/angular/angular/pull/52958)) | +| [49dca36880](https://github.com/angular/angular/commit/49dca36880a1c1c394533e8a94db9c5ef412ebd2) | fix | nested for loops incorrectly calculating computed variables ([#52931](https://github.com/angular/angular/pull/52931)) | +| [f01b7183d2](https://github.com/angular/angular/commit/f01b7183d2064f41c0f5e30ee976cc91c15e06c5) | fix | produce placeholder for blocks in i18n bundles ([#52958](https://github.com/angular/angular/pull/52958)) | +### compiler-cli +| Commit | Type | Description | +| -- | -- | -- | +| [f671f86ac2](https://github.com/angular/angular/commit/f671f86ac28d434b2fd492ef005749fe0275ece9) | fix | add diagnostic for control flow that prevents content projection ([#52726](https://github.com/angular/angular/pull/52726)) | +### core +| Commit | Type | Description | +| -- | -- | -- | +| [db1a8ebdb4](https://github.com/angular/angular/commit/db1a8ebdb4da8673107ba4ba08c42d484b733c03) | fix | cleanup loading promise when no dependencies are defined ([#53031](https://github.com/angular/angular/pull/53031)) | +| [31a1575334](https://github.com/angular/angular/commit/31a1575334ef78822d947ed858d8365ca5665f2f) | fix | handle local refs when `getDeferBlocks` is invoked in tests ([#52973](https://github.com/angular/angular/pull/52973)) | +### migrations +| Commit | Type | Description | +| -- | -- | -- | +| [ac9cd6108f](https://github.com/angular/angular/commit/ac9cd6108f6fe25e9c7a11db9816c6e07d241515) | fix | control flow migration fails for async pipe with unboxing of observable ([#52756](https://github.com/angular/angular/pull/52756)) ([#52972](https://github.com/angular/angular/pull/52972)) | +| [13bf5b7007](https://github.com/angular/angular/commit/13bf5b700739aadb2e5a210441fb815a8501ad65) | fix | Fixes control flow migration if then else case ([#53006](https://github.com/angular/angular/pull/53006)) | +| [492ad4698a](https://github.com/angular/angular/commit/492ad4698aaef51a3d24ae90f617a2ba3fae901e) | fix | fixes migrations of nested switches in control flow ([#53010](https://github.com/angular/angular/pull/53010)) | +| [0fad36eff2](https://github.com/angular/angular/commit/0fad36eff2b228baa3b8868810d4ac86eb6db459) | fix | tweaks to formatting in control flow migration ([#53058](https://github.com/angular/angular/pull/53058)) | + + + # 17.1.0-next.0 (2023-11-15) ### compiler-cli From 33959f4beabba4c9384d469f43ba621e5abc29b6 Mon Sep 17 00:00:00 2001 From: Andrew Kushnir Date: Mon, 20 Nov 2023 19:52:05 +0000 Subject: [PATCH 14/19] release: cut the v17.1.0-next.1 release --- CHANGELOG.md | 33 +++++++++++++++++++++++++++++++++ package.json | 2 +- 2 files changed, 34 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0087fdcb65796..29044fee10b25 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,36 @@ + +# 17.1.0-next.1 (2023-11-20) +### common +| Commit | Type | Description | +| -- | -- | -- | +| [29c5416d14](https://github.com/angular/angular/commit/29c5416d14638a05a894269aa5dbe67e98754418) | fix | remove `load` on image once it fails to load ([#52990](https://github.com/angular/angular/pull/52990)) | +| [7affa57754](https://github.com/angular/angular/commit/7affa5775427e92ef6e949c879765b7c8aa172da) | fix | scan images once page is loaded ([#52991](https://github.com/angular/angular/pull/52991)) | +### compiler +| Commit | Type | Description | +| -- | -- | -- | +| [ec2d6e7b9c](https://github.com/angular/angular/commit/ec2d6e7b9c2b386247d1320ee89f8e3ac5e5a0dd) | fix | changed after checked error in for loops ([#52935](https://github.com/angular/angular/pull/52935)) | +| [406049b95e](https://github.com/angular/angular/commit/406049b95e5234f17a7a18839ac848640f53fdde) | fix | generate i18n instructions for blocks ([#52958](https://github.com/angular/angular/pull/52958)) | +| [d9d566d315](https://github.com/angular/angular/commit/d9d566d31540582d73201675d0b8ed901261669e) | fix | nested for loops incorrectly calculating computed variables ([#52931](https://github.com/angular/angular/pull/52931)) | +| [5fb707f81a](https://github.com/angular/angular/commit/5fb707f81aee43751e61d2ed0861afc9b85bc85a) | fix | produce placeholder for blocks in i18n bundles ([#52958](https://github.com/angular/angular/pull/52958)) | +### compiler-cli +| Commit | Type | Description | +| -- | -- | -- | +| [b4d022e230](https://github.com/angular/angular/commit/b4d022e230ca141b12437949d11dc384bfe5c082) | fix | add diagnostic for control flow that prevents content projection ([#52726](https://github.com/angular/angular/pull/52726)) | +### core +| Commit | Type | Description | +| -- | -- | -- | +| [ed0fbd4071](https://github.com/angular/angular/commit/ed0fbd4071339b1af22d82bac07d51c6c41790cd) | fix | cleanup loading promise when no dependencies are defined ([#53031](https://github.com/angular/angular/pull/53031)) | +| [1ce31d819b](https://github.com/angular/angular/commit/1ce31d819b2e4f4425a41f07167a6edce98e77e1) | fix | handle local refs when `getDeferBlocks` is invoked in tests ([#52973](https://github.com/angular/angular/pull/52973)) | +### migrations +| Commit | Type | Description | +| -- | -- | -- | +| [e33f6e0f1a](https://github.com/angular/angular/commit/e33f6e0f1a483cad908fa6d7376d62332797499c) | fix | control flow migration fails for async pipe with unboxing of observable ([#52756](https://github.com/angular/angular/pull/52756)) ([#52972](https://github.com/angular/angular/pull/52972)) | +| [5564d020cd](https://github.com/angular/angular/commit/5564d020cdcea8273b65cf69c45c3f935195af66) | fix | Fixes control flow migration if then else case ([#53006](https://github.com/angular/angular/pull/53006)) | +| [28f6cbf9c9](https://github.com/angular/angular/commit/28f6cbf9c91f957b4926fe34610387e1f1919d4f) | fix | fixes migrations of nested switches in control flow ([#53010](https://github.com/angular/angular/pull/53010)) | +| [e090b48bf8](https://github.com/angular/angular/commit/e090b48bf8534761d46523be57a7889a325bcdec) | fix | tweaks to formatting in control flow migration ([#53058](https://github.com/angular/angular/pull/53058)) | + + + # 17.0.4 (2023-11-20) ### common diff --git a/package.json b/package.json index 2bd82c813761d..3c799c06f3e1a 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "angular-srcs", - "version": "17.1.0-next.0", + "version": "17.1.0-next.1", "private": true, "description": "Angular - a web framework for modern web apps", "homepage": "https://github.com/angular/angular", From d9a1528265325ebf475c2d08990901dd92d4d062 Mon Sep 17 00:00:00 2001 From: yuki <43907589+zhysky@users.noreply.github.com> Date: Mon, 20 Nov 2023 04:00:14 +0000 Subject: [PATCH 15/19] docs: fix packages\core\src\application_init.ts (#53040) PR Close #53040 --- packages/core/src/application_init.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/core/src/application_init.ts b/packages/core/src/application_init.ts index 395f102b05d59..a55692f310af6 100644 --- a/packages/core/src/application_init.ts +++ b/packages/core/src/application_init.ts @@ -121,7 +121,7 @@ import {isPromise, isSubscribable} from './util/lang'; * provideHttpClient(), * { * provide: APP_INITIALIZER, - * useFactory: initializeApp, + * useFactory: initializeAppFactory, * multi: true, * deps: [HttpClient], * }, From e00ae2d07a14e1001b38833ece0eed3f58ab08f0 Mon Sep 17 00:00:00 2001 From: Matthieu Riegler Date: Sun, 19 Nov 2023 17:58:44 +0100 Subject: [PATCH 16/19] refactor(core): replace `runInContext` by `runInInjectionContext` (#53035) Saves a few bytes since function names can be mangled. PR Close #53035 --- packages/common/http/src/interceptor.ts | 4 ++-- packages/common/http/src/jsonp.ts | 5 +++-- packages/common/http/src/xsrf.ts | 5 +++-- .../bundling/router/bundle.golden_symbols.json | 3 +++ packages/core/testing/src/test_bed.ts | 3 ++- packages/router/src/operators/check_guards.ts | 16 +++++++++------- packages/router/src/operators/resolve_data.ts | 4 ++-- packages/router/src/provide_router.ts | 4 ++-- 8 files changed, 26 insertions(+), 18 deletions(-) diff --git a/packages/common/http/src/interceptor.ts b/packages/common/http/src/interceptor.ts index 8d83eb5f5a2fb..73ad2cf29d9f1 100644 --- a/packages/common/http/src/interceptor.ts +++ b/packages/common/http/src/interceptor.ts @@ -7,7 +7,7 @@ */ import {isPlatformServer} from '@angular/common'; -import {EnvironmentInjector, inject, Injectable, InjectionToken, PLATFORM_ID, ɵConsole as Console, ɵformatRuntimeError as formatRuntimeError, ɵInitialRenderPendingTasks as InitialRenderPendingTasks} from '@angular/core'; +import {EnvironmentInjector, inject, Injectable, InjectionToken, PLATFORM_ID, runInInjectionContext, ɵConsole as Console, ɵformatRuntimeError as formatRuntimeError, ɵInitialRenderPendingTasks as InitialRenderPendingTasks} from '@angular/core'; import {Observable} from 'rxjs'; import {finalize} from 'rxjs/operators'; @@ -162,7 +162,7 @@ function chainedInterceptorFn( chainTailFn: ChainedInterceptorFn, interceptorFn: HttpInterceptorFn, injector: EnvironmentInjector): ChainedInterceptorFn { // clang-format off - return (initialRequest, finalHandlerFn) => injector.runInContext(() => + return (initialRequest, finalHandlerFn) => runInInjectionContext(injector, () => interceptorFn( initialRequest, downstreamRequest => chainTailFn(downstreamRequest, finalHandlerFn) diff --git a/packages/common/http/src/jsonp.ts b/packages/common/http/src/jsonp.ts index 2ceaef1e4d3dd..9f09dd900c591 100644 --- a/packages/common/http/src/jsonp.ts +++ b/packages/common/http/src/jsonp.ts @@ -7,7 +7,7 @@ */ import {DOCUMENT} from '@angular/common'; -import {EnvironmentInjector, Inject, inject, Injectable} from '@angular/core'; +import {EnvironmentInjector, Inject, inject, Injectable, runInInjectionContext} from '@angular/core'; import {Observable, Observer} from 'rxjs'; import {HttpBackend, HttpHandler} from './backend'; @@ -278,7 +278,8 @@ export class JsonpInterceptor { * @returns An observable of the event stream. */ intercept(initialRequest: HttpRequest, next: HttpHandler): Observable> { - return this.injector.runInContext( + return runInInjectionContext( + this.injector, () => jsonpInterceptorFn( initialRequest, downstreamRequest => next.handle(downstreamRequest))); } diff --git a/packages/common/http/src/xsrf.ts b/packages/common/http/src/xsrf.ts index 67db8dcec9542..2ea3cb24a90b7 100644 --- a/packages/common/http/src/xsrf.ts +++ b/packages/common/http/src/xsrf.ts @@ -7,7 +7,7 @@ */ import {DOCUMENT, ɵparseCookieValue as parseCookieValue} from '@angular/common'; -import {EnvironmentInjector, Inject, inject, Injectable, InjectionToken, PLATFORM_ID} from '@angular/core'; +import {EnvironmentInjector, Inject, inject, Injectable, InjectionToken, PLATFORM_ID, runInInjectionContext} from '@angular/core'; import {Observable} from 'rxjs'; import {HttpHandler} from './backend'; @@ -104,7 +104,8 @@ export class HttpXsrfInterceptor implements HttpInterceptor { constructor(private injector: EnvironmentInjector) {} intercept(initialRequest: HttpRequest, next: HttpHandler): Observable> { - return this.injector.runInContext( + return runInInjectionContext( + this.injector, () => xsrfInterceptorFn(initialRequest, downstreamRequest => next.handle(downstreamRequest))); } diff --git a/packages/core/test/bundling/router/bundle.golden_symbols.json b/packages/core/test/bundling/router/bundle.golden_symbols.json index 3ca528e5ac3cb..c350388f4765a 100644 --- a/packages/core/test/bundling/router/bundle.golden_symbols.json +++ b/packages/core/test/bundling/router/bundle.golden_symbols.json @@ -1898,6 +1898,9 @@ { "name": "routes" }, + { + "name": "runInInjectionContext" + }, { "name": "rxSubscriber" }, diff --git a/packages/core/testing/src/test_bed.ts b/packages/core/testing/src/test_bed.ts index 711d935b3b385..a1f25f6c80e96 100644 --- a/packages/core/testing/src/test_bed.ts +++ b/packages/core/testing/src/test_bed.ts @@ -23,6 +23,7 @@ import { Pipe, PlatformRef, ProviderToken, + runInInjectionContext, Type, ɵconvertToBitFlags as convertToBitFlags, ɵDeferBlockBehavior as DeferBlockBehavior, @@ -557,7 +558,7 @@ export class TestBedImpl implements TestBed { } runInInjectionContext(fn: () => T): T { - return this.inject(EnvironmentInjector).runInContext(fn); + return runInInjectionContext(this.inject(EnvironmentInjector), fn); } execute(tokens: any[], fn: Function, context?: any): any { diff --git a/packages/router/src/operators/check_guards.ts b/packages/router/src/operators/check_guards.ts index 7a02d2be2e30e..d5107dbd21a4a 100644 --- a/packages/router/src/operators/check_guards.ts +++ b/packages/router/src/operators/check_guards.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ -import {EnvironmentInjector, ProviderToken} from '@angular/core'; +import {EnvironmentInjector, ProviderToken, runInInjectionContext} from '@angular/core'; import {concat, defer, from, MonoTypeOperatorFunction, Observable, of, OperatorFunction, pipe} from 'rxjs'; import {concatMap, first, map, mergeMap, tap} from 'rxjs/operators'; @@ -116,7 +116,8 @@ function runCanActivate( const guard = getTokenOrFunctionIdentity(canActivate, closestInjector); const guardVal = isCanActivate(guard) ? guard.canActivate(futureARS, futureRSS) : - closestInjector.runInContext(() => (guard as CanActivateFn)(futureARS, futureRSS)); + runInInjectionContext( + closestInjector, () => (guard as CanActivateFn)(futureARS, futureRSS)); return wrapIntoObservable(guardVal).pipe(first()); }); }); @@ -142,8 +143,8 @@ function runCanActivateChild( canActivateChild, closestInjector); const guardVal = isCanActivateChild(guard) ? guard.canActivateChild(futureARS, futureRSS) : - closestInjector.runInContext( - () => (guard as CanActivateChildFn)(futureARS, futureRSS)); + runInInjectionContext( + closestInjector, () => (guard as CanActivateChildFn)(futureARS, futureRSS)); return wrapIntoObservable(guardVal).pipe(first()); }); return of(guardsMapped).pipe(prioritizedGuardValue()); @@ -162,7 +163,8 @@ function runCanDeactivate( const guard = getTokenOrFunctionIdentity(c, closestInjector); const guardVal = isCanDeactivate(guard) ? guard.canDeactivate(component, currARS, currRSS, futureRSS) : - closestInjector.runInContext( + runInInjectionContext( + closestInjector, () => (guard as CanDeactivateFn)(component, currARS, currRSS, futureRSS)); return wrapIntoObservable(guardVal).pipe(first()); }); @@ -181,7 +183,7 @@ export function runCanLoadGuards( const guard = getTokenOrFunctionIdentity(injectionToken, injector); const guardVal = isCanLoad(guard) ? guard.canLoad(route, segments) : - injector.runInContext(() => (guard as CanLoadFn)(route, segments)); + runInInjectionContext(injector, () => (guard as CanLoadFn)(route, segments)); return wrapIntoObservable(guardVal); }); @@ -214,7 +216,7 @@ export function runCanMatchGuards( const guard = getTokenOrFunctionIdentity(injectionToken, injector); const guardVal = isCanMatch(guard) ? guard.canMatch(route, segments) : - injector.runInContext(() => (guard as CanMatchFn)(route, segments)); + runInInjectionContext(injector, () => (guard as CanMatchFn)(route, segments)); return wrapIntoObservable(guardVal); }); diff --git a/packages/router/src/operators/resolve_data.ts b/packages/router/src/operators/resolve_data.ts index 38ca2c5484267..c00ce91f25bb8 100644 --- a/packages/router/src/operators/resolve_data.ts +++ b/packages/router/src/operators/resolve_data.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ -import {EnvironmentInjector, ProviderToken} from '@angular/core'; +import {EnvironmentInjector, ProviderToken, runInInjectionContext} from '@angular/core'; import {EMPTY, from, MonoTypeOperatorFunction, Observable, of, throwError} from 'rxjs'; import {catchError, concatMap, first, map, mapTo, mergeMap, takeLast, tap} from 'rxjs/operators'; @@ -106,6 +106,6 @@ function getResolver( const resolver = getTokenOrFunctionIdentity(injectionToken, closestInjector); const resolverValue = resolver.resolve ? resolver.resolve(futureARS, futureRSS) : - closestInjector.runInContext(() => resolver(futureARS, futureRSS)); + runInInjectionContext(closestInjector, () => resolver(futureARS, futureRSS)); return wrapIntoObservable(resolverValue); } diff --git a/packages/router/src/provide_router.ts b/packages/router/src/provide_router.ts index dcb7100eebab0..fc9c84c72d21c 100644 --- a/packages/router/src/provide_router.ts +++ b/packages/router/src/provide_router.ts @@ -7,7 +7,7 @@ */ import {HashLocationStrategy, LOCATION_INITIALIZED, LocationStrategy, ViewportScroller} from '@angular/common'; -import {APP_BOOTSTRAP_LISTENER, APP_INITIALIZER, ApplicationRef, Component, ComponentRef, ENVIRONMENT_INITIALIZER, EnvironmentInjector, EnvironmentProviders, inject, InjectFlags, InjectionToken, Injector, makeEnvironmentProviders, NgZone, Provider, Type} from '@angular/core'; +import {APP_BOOTSTRAP_LISTENER, APP_INITIALIZER, ApplicationRef, Component, ComponentRef, ENVIRONMENT_INITIALIZER, EnvironmentInjector, EnvironmentProviders, inject, InjectFlags, InjectionToken, Injector, makeEnvironmentProviders, NgZone, Provider, runInInjectionContext, Type} from '@angular/core'; import {of, Subject} from 'rxjs'; import {INPUT_BINDER, RoutedComponentInputBinder} from './directives/router_outlet'; @@ -643,7 +643,7 @@ export function withNavigationErrorHandler(fn: (error: NavigationError) => void) const injector = inject(EnvironmentInjector); inject(Router).events.subscribe((e) => { if (e instanceof NavigationError) { - injector.runInContext(() => fn(e)); + runInInjectionContext(injector, () => fn(e)); } }); } From b35c6731e51de9c33707010fc780cbaa559be6c3 Mon Sep 17 00:00:00 2001 From: Andrew Scott Date: Thu, 16 Nov 2023 16:45:13 -0800 Subject: [PATCH 17/19] fix(core): Reattached views that are dirty from a signal update should refresh (#53001) Related to #52928 but `updateAncestorTraversalFlagsOnAttach` is called on view insertion and _should_ have made that work for views dirty from signals but it wasn't updated to read the `dirty` flag when we changed it from sharing the `RefreshView` flag. For #52928, we've traditionally worked under the assumption that this is working as expected. The created view is `CheckAlways`. There is a question of whether we should automatically mark things for check when the attached view has the `Dirty` flag and/or has the `FirstLViewPass` flag set (or other flags that indicate it definitely needs to be prefreshed). PR Close #53001 --- .../render3/instructions/change_detection.ts | 5 ++-- packages/core/src/render3/util/view_utils.ts | 23 ++++++++++--------- .../change_detection_signals_in_zones_spec.ts | 16 ++++++++++++- .../bundle.golden_symbols.json | 3 +++ .../animations/bundle.golden_symbols.json | 3 +++ .../cyclic_import/bundle.golden_symbols.json | 3 +++ .../bundling/defer/bundle.golden_symbols.json | 3 +++ .../forms_reactive/bundle.golden_symbols.json | 3 +++ .../bundle.golden_symbols.json | 3 +++ .../hello_world/bundle.golden_symbols.json | 3 +++ .../hydration/bundle.golden_symbols.json | 3 +++ .../router/bundle.golden_symbols.json | 3 +++ .../bundle.golden_symbols.json | 3 +++ .../bundling/todo/bundle.golden_symbols.json | 3 +++ 14 files changed, 62 insertions(+), 15 deletions(-) diff --git a/packages/core/src/render3/instructions/change_detection.ts b/packages/core/src/render3/instructions/change_detection.ts index 6c7512302c48f..d17e3492a724d 100644 --- a/packages/core/src/render3/instructions/change_detection.ts +++ b/packages/core/src/render3/instructions/change_detection.ts @@ -18,7 +18,7 @@ import {CONTEXT, EFFECTS_TO_SCHEDULE, ENVIRONMENT, FLAGS, InitPhaseState, LView, import {getOrBorrowReactiveLViewConsumer, maybeReturnReactiveLViewConsumer, ReactiveLViewConsumer} from '../reactive_lview_consumer'; import {enterView, isInCheckNoChangesMode, leaveView, setBindingIndex, setIsInCheckNoChangesMode} from '../state'; import {getFirstLContainer, getNextLContainer} from '../util/view_traversal_utils'; -import {getComponentLViewByIndex, isCreationMode, markAncestorsForTraversal, markViewForRefresh, resetPreOrderHookFlags, viewAttachedToChangeDetector} from '../util/view_utils'; +import {getComponentLViewByIndex, isCreationMode, markAncestorsForTraversal, markViewForRefresh, requiresRefreshOrTraversal, resetPreOrderHookFlags, viewAttachedToChangeDetector} from '../util/view_utils'; import {executeTemplate, executeViewQueryFn, handleError, processHostBindingOpCodes, refreshContentQueries} from './shared'; @@ -72,8 +72,7 @@ function detectChangesInViewWhileDirty(lView: LView) { // descendants views that need to be refreshed due to re-dirtying during the change detection // run, detect changes on the view again. We run change detection in `Targeted` mode to only // refresh views with the `RefreshView` flag. - while (lView[FLAGS] & (LViewFlags.RefreshView | LViewFlags.HasChildViewsToRefresh) || - lView[REACTIVE_TEMPLATE_CONSUMER]?.dirty) { + while (requiresRefreshOrTraversal(lView)) { if (retries === MAXIMUM_REFRESH_RERUNS) { throw new RuntimeError( RuntimeErrorCode.INFINITE_CHANGE_DETECTION, diff --git a/packages/core/src/render3/util/view_utils.ts b/packages/core/src/render3/util/view_utils.ts index 7520b2a82eff5..59f2656e7bae5 100644 --- a/packages/core/src/render3/util/view_utils.ts +++ b/packages/core/src/render3/util/view_utils.ts @@ -13,7 +13,7 @@ import {LContainer, LContainerFlags, TYPE} from '../interfaces/container'; import {TConstants, TNode} from '../interfaces/node'; import {RNode} from '../interfaces/renderer_dom'; import {isLContainer, isLView} from '../interfaces/type_checks'; -import {DECLARATION_VIEW, FLAGS, HEADER_OFFSET, HOST, LView, LViewFlags, ON_DESTROY_HOOKS, PARENT, PREORDER_HOOK_FLAGS, PreOrderHookFlags, TData, TView} from '../interfaces/view'; +import {DECLARATION_VIEW, FLAGS, HEADER_OFFSET, HOST, LView, LViewFlags, ON_DESTROY_HOOKS, PARENT, PREORDER_HOOK_FLAGS, PreOrderHookFlags, REACTIVE_TEMPLATE_CONSUMER, TData, TView} from '../interfaces/view'; @@ -195,21 +195,22 @@ export function walkUpViews(nestingLevel: number, currentView: LView): LView { return currentView; } +export function requiresRefreshOrTraversal(lView: LView) { + return lView[FLAGS] & (LViewFlags.RefreshView | LViewFlags.HasChildViewsToRefresh) || + lView[REACTIVE_TEMPLATE_CONSUMER]?.dirty; +} + /** - * Updates the `DESCENDANT_VIEWS_TO_REFRESH` counter on the parents of the `LView` as well as the - * parents above that whose - * 1. counter goes from 0 to 1, indicating that there is a new child that has a view to refresh - * or - * 2. counter goes from 1 to 0, indicating there are no more descendant views to refresh - * When attaching/re-attaching an `LView` to the change detection tree, we need to ensure that the - * views above it are traversed during change detection if this one is marked for refresh or has - * some child or descendant that needs to be refreshed. + * Updates the `HasChildViewsToRefresh` flag on the parents of the `LView` as well as the + * parents above. */ export function updateAncestorTraversalFlagsOnAttach(lView: LView) { - if (lView[FLAGS] & (LViewFlags.RefreshView | LViewFlags.HasChildViewsToRefresh)) { - markAncestorsForTraversal(lView); + if (!requiresRefreshOrTraversal(lView)) { + return; } + + markAncestorsForTraversal(lView); } /** diff --git a/packages/core/test/acceptance/change_detection_signals_in_zones_spec.ts b/packages/core/test/acceptance/change_detection_signals_in_zones_spec.ts index 9195f0460fa8f..dd961f98f4384 100644 --- a/packages/core/test/acceptance/change_detection_signals_in_zones_spec.ts +++ b/packages/core/test/acceptance/change_detection_signals_in_zones_spec.ts @@ -793,7 +793,9 @@ describe('OnPush components with signals', () => { @Component({ selector: 'on-push-parent', - template: `{{incrementChecks()}}`, + template: ` + + {{incrementChecks()}}`, changeDetection: ChangeDetectionStrategy.OnPush, standalone: true, imports: [SignalComponent], @@ -827,6 +829,18 @@ describe('OnPush components with signals', () => { expect(trim(fixture.nativeElement.textContent)).toEqual('initial'); }); + it('refreshes when reattached if already dirty', () => { + const fixture = TestBed.createComponent(OnPushParent); + fixture.detectChanges(); + fixture.componentInstance.signalChild.value.set('new'); + fixture.componentInstance.signalChild.cdr.detach(); + fixture.detectChanges(); + expect(trim(fixture.nativeElement.textContent)).toEqual('initial'); + fixture.componentInstance.signalChild.cdr.reattach(); + fixture.detectChanges(); + expect(trim(fixture.nativeElement.textContent)).toEqual('new'); + }); + // Note: Design decision for signals because that's how the hooks work today // We have considered actually running a component's `afterViewChecked` hook if it's refreshed // in targeted mode (meaning the parent did not refresh) and could change this decision. diff --git a/packages/core/test/bundling/animations-standalone/bundle.golden_symbols.json b/packages/core/test/bundling/animations-standalone/bundle.golden_symbols.json index 01eaba391e910..1b0d113b526f9 100644 --- a/packages/core/test/bundling/animations-standalone/bundle.golden_symbols.json +++ b/packages/core/test/bundling/animations-standalone/bundle.golden_symbols.json @@ -1328,6 +1328,9 @@ { "name": "replacePostStylesAsPre" }, + { + "name": "requiresRefreshOrTraversal" + }, { "name": "resetPreOrderHookFlags" }, diff --git a/packages/core/test/bundling/animations/bundle.golden_symbols.json b/packages/core/test/bundling/animations/bundle.golden_symbols.json index 7c13020110545..89281f76307a7 100644 --- a/packages/core/test/bundling/animations/bundle.golden_symbols.json +++ b/packages/core/test/bundling/animations/bundle.golden_symbols.json @@ -1400,6 +1400,9 @@ { "name": "replacePostStylesAsPre" }, + { + "name": "requiresRefreshOrTraversal" + }, { "name": "resetPreOrderHookFlags" }, diff --git a/packages/core/test/bundling/cyclic_import/bundle.golden_symbols.json b/packages/core/test/bundling/cyclic_import/bundle.golden_symbols.json index db31761dc1b5f..e8dad0f93977a 100644 --- a/packages/core/test/bundling/cyclic_import/bundle.golden_symbols.json +++ b/packages/core/test/bundling/cyclic_import/bundle.golden_symbols.json @@ -1112,6 +1112,9 @@ { "name": "renderView" }, + { + "name": "requiresRefreshOrTraversal" + }, { "name": "resetPreOrderHookFlags" }, diff --git a/packages/core/test/bundling/defer/bundle.golden_symbols.json b/packages/core/test/bundling/defer/bundle.golden_symbols.json index f4e3b0a5e7916..75806d3e6f10d 100644 --- a/packages/core/test/bundling/defer/bundle.golden_symbols.json +++ b/packages/core/test/bundling/defer/bundle.golden_symbols.json @@ -2249,6 +2249,9 @@ { "name": "renderView" }, + { + "name": "requiresRefreshOrTraversal" + }, { "name": "resetPreOrderHookFlags" }, diff --git a/packages/core/test/bundling/forms_reactive/bundle.golden_symbols.json b/packages/core/test/bundling/forms_reactive/bundle.golden_symbols.json index e4313b3e406c4..c8b8cc508bc22 100644 --- a/packages/core/test/bundling/forms_reactive/bundle.golden_symbols.json +++ b/packages/core/test/bundling/forms_reactive/bundle.golden_symbols.json @@ -1553,6 +1553,9 @@ { "name": "renderView" }, + { + "name": "requiresRefreshOrTraversal" + }, { "name": "resetPreOrderHookFlags" }, diff --git a/packages/core/test/bundling/forms_template_driven/bundle.golden_symbols.json b/packages/core/test/bundling/forms_template_driven/bundle.golden_symbols.json index 195dff896bbf0..b1d84910400dd 100644 --- a/packages/core/test/bundling/forms_template_driven/bundle.golden_symbols.json +++ b/packages/core/test/bundling/forms_template_driven/bundle.golden_symbols.json @@ -1523,6 +1523,9 @@ { "name": "requiredValidator" }, + { + "name": "requiresRefreshOrTraversal" + }, { "name": "resetPreOrderHookFlags" }, diff --git a/packages/core/test/bundling/hello_world/bundle.golden_symbols.json b/packages/core/test/bundling/hello_world/bundle.golden_symbols.json index 25d0847c0e3d5..74352ba37f0bf 100644 --- a/packages/core/test/bundling/hello_world/bundle.golden_symbols.json +++ b/packages/core/test/bundling/hello_world/bundle.golden_symbols.json @@ -881,6 +881,9 @@ { "name": "renderView" }, + { + "name": "requiresRefreshOrTraversal" + }, { "name": "resetPreOrderHookFlags" }, diff --git a/packages/core/test/bundling/hydration/bundle.golden_symbols.json b/packages/core/test/bundling/hydration/bundle.golden_symbols.json index 8d0425bdf8880..6b3c37c6e43f3 100644 --- a/packages/core/test/bundling/hydration/bundle.golden_symbols.json +++ b/packages/core/test/bundling/hydration/bundle.golden_symbols.json @@ -1220,6 +1220,9 @@ { "name": "renderView" }, + { + "name": "requiresRefreshOrTraversal" + }, { "name": "resetPreOrderHookFlags" }, diff --git a/packages/core/test/bundling/router/bundle.golden_symbols.json b/packages/core/test/bundling/router/bundle.golden_symbols.json index c350388f4765a..a6a42c39af7b5 100644 --- a/packages/core/test/bundling/router/bundle.golden_symbols.json +++ b/packages/core/test/bundling/router/bundle.golden_symbols.json @@ -1880,6 +1880,9 @@ { "name": "replaceSegment" }, + { + "name": "requiresRefreshOrTraversal" + }, { "name": "resetPreOrderHookFlags" }, diff --git a/packages/core/test/bundling/standalone_bootstrap/bundle.golden_symbols.json b/packages/core/test/bundling/standalone_bootstrap/bundle.golden_symbols.json index 4f00f629567a6..ab1fddd2ac42b 100644 --- a/packages/core/test/bundling/standalone_bootstrap/bundle.golden_symbols.json +++ b/packages/core/test/bundling/standalone_bootstrap/bundle.golden_symbols.json @@ -977,6 +977,9 @@ { "name": "renderView" }, + { + "name": "requiresRefreshOrTraversal" + }, { "name": "resetPreOrderHookFlags" }, diff --git a/packages/core/test/bundling/todo/bundle.golden_symbols.json b/packages/core/test/bundling/todo/bundle.golden_symbols.json index 17dfc360fa33f..cf86883f5d0f0 100644 --- a/packages/core/test/bundling/todo/bundle.golden_symbols.json +++ b/packages/core/test/bundling/todo/bundle.golden_symbols.json @@ -1334,6 +1334,9 @@ { "name": "renderView" }, + { + "name": "requiresRefreshOrTraversal" + }, { "name": "resetPreOrderHookFlags" }, From 29e0834c4deecfa8bf384b5e4359796c8123afcd Mon Sep 17 00:00:00 2001 From: Leonel Franchelli Date: Wed, 15 Nov 2023 11:26:28 -0300 Subject: [PATCH 18/19] fix(router): Resolvers in different parts of the route tree should be able to execute together (#52934) The following commit accidentally broken execution of resolvers when two resolvers appear in different parts of the tree and do not share a https://github.com/angular/angular/commit/327896606832bf6fbfc8f23989755123028136a8 This happens when there are secondary routes. This test ensures that all routes with resolves are run. fixes #52892 PR Close #52934 --- .../router/bundle.golden_symbols.json | 3 -- packages/router/src/operators/resolve_data.ts | 28 ++++++++------ .../test/operators/resolve_data.spec.ts | 38 +++++++++++++++++++ 3 files changed, 54 insertions(+), 15 deletions(-) diff --git a/packages/core/test/bundling/router/bundle.golden_symbols.json b/packages/core/test/bundling/router/bundle.golden_symbols.json index a6a42c39af7b5..9fc2d84eafbd8 100644 --- a/packages/core/test/bundling/router/bundle.golden_symbols.json +++ b/packages/core/test/bundling/router/bundle.golden_symbols.json @@ -1886,9 +1886,6 @@ { "name": "resetPreOrderHookFlags" }, - { - "name": "resolveData" - }, { "name": "resolveForwardRef" }, diff --git a/packages/router/src/operators/resolve_data.ts b/packages/router/src/operators/resolve_data.ts index c00ce91f25bb8..32279738d749e 100644 --- a/packages/router/src/operators/resolve_data.ts +++ b/packages/router/src/operators/resolve_data.ts @@ -28,21 +28,25 @@ export function resolveData( if (!canActivateChecks.length) { return of(t); } - const routesWithResolversToRun = canActivateChecks.map(check => check.route); - const routesWithResolversSet = new Set(routesWithResolversToRun); - const routesNeedingDataUpdates = - // List all ActivatedRoutes in an array, starting from the parent of the first route to run - // resolvers. We go from the parent because the first route might have siblings that also - // run resolvers. - flattenRouteTree(routesWithResolversToRun[0].parent!) - // Remove the parent from the list -- we do not need to recompute its inherited data - // because no resolvers at or above it run. - .slice(1); + // Iterating a Set in javascript happens in insertion order so it is safe to use a `Set` to + // preserve the correct order that the resolvers should run in. + // https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Set#description + const routesWithResolversToRun = new Set(canActivateChecks.map(check => check.route)); + const routesNeedingDataUpdates = new Set(); + for (const route of routesWithResolversToRun) { + if (routesNeedingDataUpdates.has(route)) { + continue; + } + // All children under the route with a resolver to run need to recompute inherited data. + for (const newRoute of flattenRouteTree(route)) { + routesNeedingDataUpdates.add(newRoute); + } + } let routesProcessed = 0; return from(routesNeedingDataUpdates) .pipe( concatMap(route => { - if (routesWithResolversSet.has(route)) { + if (routesWithResolversToRun.has(route)) { return runResolve(route, targetSnapshot!, paramsInheritanceStrategy, injector); } else { route.data = getInherited(route, route.parent, paramsInheritanceStrategy).resolve; @@ -51,7 +55,7 @@ export function resolveData( }), tap(() => routesProcessed++), takeLast(1), - mergeMap(_ => routesProcessed === routesNeedingDataUpdates.length ? of(t) : EMPTY), + mergeMap(_ => routesProcessed === routesNeedingDataUpdates.size ? of(t) : EMPTY), ); }); } diff --git a/packages/router/test/operators/resolve_data.spec.ts b/packages/router/test/operators/resolve_data.spec.ts index 98e0564f1bf7b..305f9ac82b832 100644 --- a/packages/router/test/operators/resolve_data.spec.ts +++ b/packages/router/test/operators/resolve_data.spec.ts @@ -46,6 +46,44 @@ describe('resolveData operator', () => { expect(TestBed.inject(Router).url).toEqual('/'); }); + it('should run resolvers in different parts of the tree', async () => { + let value = 0; + let bValue = 0; + TestBed.configureTestingModule({ + providers: [provideRouter([ + { + path: 'a', + runGuardsAndResolvers: () => false, + children: [{ + path: '', + resolve: {d0: () => ++value}, + runGuardsAndResolvers: 'always', + children: [], + }], + }, + { + path: 'b', + outlet: 'aux', + runGuardsAndResolvers: () => false, + children: [{ + path: '', + resolve: {d1: () => ++bValue}, + runGuardsAndResolvers: 'always', + children: [], + }] + }, + ])] + }); + const router = TestBed.inject(Router); + const harness = await RouterTestingHarness.create('/a(aux:b)'); + expect(router.routerState.root.children[0]?.firstChild?.snapshot.data).toEqual({d0: 1}); + expect(router.routerState.root.children[1]?.firstChild?.snapshot.data).toEqual({d1: 1}); + + await harness.navigateByUrl('/a(aux:b)#new'); + expect(router.routerState.root.children[0]?.firstChild?.snapshot.data).toEqual({d0: 2}); + expect(router.routerState.root.children[1]?.firstChild?.snapshot.data).toEqual({d1: 2}); + }); + it('should update children inherited data when resolvers run', async () => { let value = 0; TestBed.configureTestingModule({ From c7c7ea9813f6dcf91c096bb37d36bfe0c715a04f Mon Sep 17 00:00:00 2001 From: Kristiyan Kostadinov Date: Fri, 17 Nov 2023 21:23:28 +0100 Subject: [PATCH 19/19] fix(core): inherit host directives (#52992) Adds support for inheriting host directives from the parent class. This is consistent with how we inherit other features like host bindings. Fixes #51203. PR Close #52992 --- .../src/ngtsc/metadata/src/inheritance.ts | 8 +- .../test/ngtsc/template_typecheck_spec.ts | 97 ++++++++++ .../compiler/src/render3/view/compiler.ts | 10 +- .../features/host_directives_feature.ts | 15 +- .../test/acceptance/host_directives_spec.ts | 181 ++++++++++++++++++ 5 files changed, 302 insertions(+), 9 deletions(-) diff --git a/packages/compiler-cli/src/ngtsc/metadata/src/inheritance.ts b/packages/compiler-cli/src/ngtsc/metadata/src/inheritance.ts index 22e99ad52305c..6d3c845f71b19 100644 --- a/packages/compiler-cli/src/ngtsc/metadata/src/inheritance.ts +++ b/packages/compiler-cli/src/ngtsc/metadata/src/inheritance.ts @@ -9,7 +9,7 @@ import {Reference} from '../../imports'; import {ClassDeclaration} from '../../reflection'; -import {DirectiveMeta, InputMapping, MetadataReader} from './api'; +import {DirectiveMeta, HostDirectiveMeta, InputMapping, MetadataReader} from './api'; import {ClassPropertyMapping, ClassPropertyName} from './property_mapping'; /** @@ -34,6 +34,7 @@ export function flattenInheritedDirectiveMetadata( const undeclaredInputFields = new Set(); const restrictedInputFields = new Set(); const stringLiteralInputFields = new Set(); + let hostDirectives: HostDirectiveMeta[]|null = null; let isDynamic = false; let inputs = ClassPropertyMapping.empty(); let outputs = ClassPropertyMapping.empty(); @@ -69,6 +70,10 @@ export function flattenInheritedDirectiveMetadata( for (const field of meta.stringLiteralInputFields) { stringLiteralInputFields.add(field); } + if (meta.hostDirectives !== null && meta.hostDirectives.length > 0) { + hostDirectives ??= []; + hostDirectives.push(...meta.hostDirectives); + } }; addMetadata(topMeta); @@ -83,5 +88,6 @@ export function flattenInheritedDirectiveMetadata( stringLiteralInputFields, baseClass: isDynamic ? 'dynamic' : null, isStructural, + hostDirectives, }; } diff --git a/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts b/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts index e3a9291589bec..8aaae84a86fe5 100644 --- a/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts +++ b/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts @@ -3622,6 +3622,103 @@ suppress expect(ts.flattenDiagnosticMessageText(diags[0].messageText, '')) .toContain('HostBindDirective'); }); + + it('should check bindings to inherited host directive inputs', () => { + env.write('test.ts', ` + import {Component, Directive, NgModule, Input} from '@angular/core'; + + @Directive({ + standalone: true, + }) + class HostDir { + @Input() input: number; + @Input() otherInput: string; + } + + @Directive({ + hostDirectives: [{directive: HostDir, inputs: ['input', 'otherInput: alias']}] + }) + class Parent {} + + @Directive({selector: '[dir]'}) + class Dir extends Parent {} + + @Component({ + selector: 'test', + template: '
', + }) + class TestCmp { + person: { + name: string; + age: number; + }; + } + + @NgModule({ + declarations: [TestCmp, Dir], + }) + class Module {} + `); + + + const messages = env.driveDiagnostics().map(d => d.messageText); + + expect(messages).toEqual([ + `Type 'string' is not assignable to type 'number'.`, + `Type 'number' is not assignable to type 'string'.` + ]); + }); + + it('should check bindings to inherited host directive outputs', () => { + env.write('test.ts', ` + import {Component, Directive, NgModule, Output, EventEmitter} from '@angular/core'; + + @Directive({ + standalone: true, + }) + class HostDir { + @Output() stringEvent = new EventEmitter(); + @Output() numberEvent = new EventEmitter(); + } + + @Directive({ + hostDirectives: [ + {directive: HostDir, outputs: ['stringEvent', 'numberEvent: numberAlias']} + ] + }) + class Parent {} + + @Directive({selector: '[dir]'}) + class Dir extends Parent {} + + @Component({ + selector: 'test', + template: \` +
+ \`, + }) + class TestCmp { + handleStringEvent(event: string): void {} + handleNumberEvent(event: number): void {} + } + + @NgModule({ + declarations: [TestCmp, Dir], + }) + class Module {} + `); + + + const messages = env.driveDiagnostics().map(d => d.messageText); + + expect(messages).toEqual([ + `Argument of type 'number' is not assignable to parameter of type 'string'.`, + `Argument of type 'string' is not assignable to parameter of type 'number'.` + ]); + }); }); describe('deferred blocks', () => { diff --git a/packages/compiler/src/render3/view/compiler.ts b/packages/compiler/src/render3/view/compiler.ts index 04bcd6df14511..f0141d8e1984b 100644 --- a/packages/compiler/src/render3/view/compiler.ts +++ b/packages/compiler/src/render3/view/compiler.ts @@ -118,6 +118,12 @@ function addFeatures( break; } } + // Note: host directives feature needs to be inserted before the + // inheritance feature to ensure the correct execution order. + if (meta.hostDirectives?.length) { + features.push(o.importExpr(R3.HostDirectivesFeature).callFn([createHostDirectivesFeatureArg( + meta.hostDirectives)])); + } if (meta.usesInheritance) { features.push(o.importExpr(R3.InheritDefinitionFeature)); } @@ -131,10 +137,6 @@ function addFeatures( if (meta.hasOwnProperty('template') && meta.isStandalone) { features.push(o.importExpr(R3.StandaloneFeature)); } - if (meta.hostDirectives?.length) { - features.push(o.importExpr(R3.HostDirectivesFeature).callFn([createHostDirectivesFeatureArg( - meta.hostDirectives)])); - } if (features.length) { definitionMap.set('features', o.literalArr(features)); } diff --git a/packages/core/src/render3/features/host_directives_feature.ts b/packages/core/src/render3/features/host_directives_feature.ts index f63f66e5703fd..33fa8dd920e1c 100644 --- a/packages/core/src/render3/features/host_directives_feature.ts +++ b/packages/core/src/render3/features/host_directives_feature.ts @@ -11,7 +11,7 @@ import {Type} from '../../interface/type'; import {assertEqual} from '../../util/assert'; import {EMPTY_OBJ} from '../../util/empty'; import {getComponentDef, getDirectiveDef} from '../definition'; -import {DirectiveDef, HostDirectiveBindingMap, HostDirectiveDef, HostDirectiveDefs} from '../interfaces/definition'; +import {DirectiveDef, DirectiveDefFeature, HostDirectiveBindingMap, HostDirectiveDef, HostDirectiveDefs} from '../interfaces/definition'; /** Values that can be used to define a host directive through the `HostDirectivesFeature`. */ type HostDirectiveConfig = Type|{ @@ -42,9 +42,8 @@ type HostDirectiveConfig = Type|{ */ export function ɵɵHostDirectivesFeature(rawHostDirectives: HostDirectiveConfig[]| (() => HostDirectiveConfig[])) { - return (definition: DirectiveDef) => { - definition.findHostDirectiveDefs = findHostDirectiveDefs; - definition.hostDirectives = + const feature: DirectiveDefFeature = (definition: DirectiveDef) => { + const resolved = (Array.isArray(rawHostDirectives) ? rawHostDirectives : rawHostDirectives()).map(dir => { return typeof dir === 'function' ? {directive: resolveForwardRef(dir), inputs: EMPTY_OBJ, outputs: EMPTY_OBJ} : @@ -54,7 +53,15 @@ export function ɵɵHostDirectivesFeature(rawHostDirectives: HostDirectiveConfig outputs: bindingArrayToMap(dir.outputs) }; }); + if (definition.hostDirectives === null) { + definition.findHostDirectiveDefs = findHostDirectiveDefs; + definition.hostDirectives = resolved; + } else { + definition.hostDirectives.unshift(...resolved); + } }; + feature.ngInherit = true; + return feature; } function findHostDirectiveDefs( diff --git a/packages/core/test/acceptance/host_directives_spec.ts b/packages/core/test/acceptance/host_directives_spec.ts index 89636c60bc4bb..5cb8f7b33778c 100644 --- a/packages/core/test/acceptance/host_directives_spec.ts +++ b/packages/core/test/acceptance/host_directives_spec.ts @@ -304,6 +304,95 @@ describe('host directives', () => { expect(fixture.nativeElement.textContent).toContain('FirstHost | SecondHost'); }); + it('should execute inherited host directives in the correct order', () => { + const logs: string[] = []; + + @Directive({standalone: true}) + class HostGrandparent_1 { + constructor() { + logs.push('HostGrandparent_1'); + } + } + + @Directive({standalone: true}) + class HostGrandparent_2 { + constructor() { + logs.push('HostGrandparent_2'); + } + } + + @Directive({standalone: true, hostDirectives: [HostGrandparent_1, HostGrandparent_2]}) + class Grandparent { + constructor() { + logs.push('Grandparent'); + } + } + + @Directive({standalone: true}) + class HostParent_1 { + constructor() { + logs.push('HostParent_1'); + } + } + + @Directive({standalone: true}) + class HostParent_2 { + constructor() { + logs.push('HostParent_2'); + } + } + + @Directive({standalone: true, hostDirectives: [HostParent_1, HostParent_2]}) + class Parent extends Grandparent { + constructor() { + super(); + logs.push('Parent'); + } + } + + @Directive({standalone: true}) + class HostDir_1 { + constructor() { + logs.push('HostDir_1'); + } + } + + @Directive({standalone: true}) + class HostDir_2 { + constructor() { + logs.push('HostDir_2'); + } + } + + @Directive({selector: '[dir]', hostDirectives: [HostDir_1, HostDir_2]}) + class Dir extends Parent { + constructor() { + super(); + logs.push('Dir'); + } + } + + @Component({template: '
'}) + class App { + } + + TestBed.configureTestingModule({declarations: [App, Dir]}); + const fixture = TestBed.createComponent(App); + fixture.detectChanges(); + + expect(logs).toEqual([ + 'HostGrandparent_1', + 'HostGrandparent_2', + 'HostParent_1', + 'HostParent_2', + 'HostDir_1', + 'HostDir_2', + 'Grandparent', + 'Parent', + 'Dir', + ]); + }); + describe('lifecycle hooks', () => { it('should invoke lifecycle hooks from the host directives', () => { const logs: string[] = []; @@ -1329,6 +1418,40 @@ describe('host directives', () => { expect(fixture.componentInstance.spy).toHaveBeenCalledWith('FirstHostDir'); expect(fixture.componentInstance.spy).toHaveBeenCalledWith('SecondHostDir'); }); + + it('should emit to an output of an inherited host directive that has been exposed', () => { + @Directive({standalone: true, host: {'(click)': 'hasBeenClicked.emit("hello")'}}) + class HostDir { + @Output() hasBeenClicked = new EventEmitter(); + } + + @Directive({ + hostDirectives: [{ + directive: HostDir, + outputs: ['hasBeenClicked'], + }] + }) + class Parent { + } + + @Directive({selector: '[dir]'}) + class Dir extends Parent { + } + + @Component({template: ''}) + class App { + spy = jasmine.createSpy('click spy'); + } + + TestBed.configureTestingModule({declarations: [App, Dir]}); + const fixture = TestBed.createComponent(App); + fixture.detectChanges(); + + fixture.nativeElement.querySelector('button').click(); + fixture.detectChanges(); + + expect(fixture.componentInstance.spy).toHaveBeenCalledOnceWith('hello'); + }); }); describe('inputs', () => { @@ -1874,6 +1997,36 @@ describe('host directives', () => { // The input on the button instance should not have been written to. expect(logs).toEqual(['spanValue']); }); + + it('should set the input of an inherited host directive that has been exposed', () => { + @Directive({standalone: true}) + class HostDir { + @Input() color?: string; + } + + @Directive({hostDirectives: [{directive: HostDir, inputs: ['color']}]}) + class Parent { + } + + @Directive({selector: '[dir]'}) + class Dir extends Parent { + } + + @Component({template: ''}) + class App { + @ViewChild(HostDir) hostDir!: HostDir; + color = 'red'; + } + + TestBed.configureTestingModule({declarations: [App, Dir]}); + const fixture = TestBed.createComponent(App); + fixture.detectChanges(); + expect(fixture.componentInstance.hostDir.color).toBe('red'); + + fixture.componentInstance.color = 'green'; + fixture.detectChanges(); + expect(fixture.componentInstance.hostDir.color).toBe('green'); + }); }); describe('ngOnChanges', () => { @@ -3318,5 +3471,33 @@ describe('host directives', () => { fixture.detectChanges(); }).not.toThrow(); }); + + it('should throw an error if a duplicate directive is inherited', () => { + @Directive({standalone: true}) + class HostDir { + } + + @Directive({standalone: true, hostDirectives: [HostDir]}) + class Grandparent { + } + + @Directive({standalone: true}) + class Parent extends Grandparent { + } + + @Directive({selector: '[dir]', hostDirectives: [HostDir]}) + class Dir extends Parent { + } + + @Component({template: '
'}) + class App { + } + + TestBed.configureTestingModule({declarations: [App, Dir]}); + + expect(() => TestBed.createComponent(App)) + .toThrowError( + 'NG0309: Directive HostDir matches multiple times on the same element. Directives can only match an element once.'); + }); }); });