From 8297a856d926a12a43c3229696b930c4cf674089 Mon Sep 17 00:00:00 2001 From: Kristiyan Kostadinov Date: Wed, 5 Feb 2025 04:55:15 -0800 Subject: [PATCH 1/2] refactor(compiler): pass more information to HMR replacement function Adjusts the code we generate for HMR so that it passes in the HMR ID and `import.meta` to the `replaceMetadata` call. This is necessary so we can do better logging of errors. --- packages/compiler-cli/test/ngtsc/hmr_spec.ts | 54 ++++++++++++------- .../compiler/src/render3/r3_hmr_compiler.ts | 24 ++++++--- packages/core/src/render3/hmr.ts | 3 ++ packages/core/test/acceptance/hmr_spec.ts | 1 + 4 files changed, 55 insertions(+), 27 deletions(-) diff --git a/packages/compiler-cli/test/ngtsc/hmr_spec.ts b/packages/compiler-cli/test/ngtsc/hmr_spec.ts index 4da69740c23f..63acd4c5290a 100644 --- a/packages/compiler-cli/test/ngtsc/hmr_spec.ts +++ b/packages/compiler-cli/test/ngtsc/hmr_spec.ts @@ -104,18 +104,19 @@ runInEachFileSystem(() => { const hmrContents = env.driveHmr('test.ts', 'Cmp'); expect(jsContents).toContain(`import * as i0 from "@angular/core";`); + expect(jsContents).toContain('const id = "test.ts%40Cmp";'); expect(jsContents).toContain('function Cmp_HmrLoad(t) {'); expect(jsContents).toContain( - 'import(/* @vite-ignore */\nnew URL("./@ng/component?c=test.ts%40Cmp&t=" + encodeURIComponent(t), import.meta.url).href)', + 'import(/* @vite-ignore */\nnew URL("./@ng/component?c=" + id + "&t=" + encodeURIComponent(t), import.meta.url).href)', ); expect(jsContents).toContain( ').then(m => m.default && i0.ɵɵreplaceMetadata(Cmp, m.default, [i0], ' + - '[Dep, transformValue, TOKEN, Component, Inject, ViewChild, Input]));', + '[Dep, transformValue, TOKEN, Component, Inject, ViewChild, Input], import.meta, id));', ); expect(jsContents).toContain('Cmp_HmrLoad(Date.now());'); expect(jsContents).toContain( 'import.meta.hot && import.meta.hot.on("angular:component-update", ' + - 'd => d.id === "test.ts%40Cmp" && Cmp_HmrLoad(d.timestamp)', + 'd => d.id === id && Cmp_HmrLoad(d.timestamp)', ); expect(hmrContents).toContain( @@ -171,18 +172,19 @@ runInEachFileSystem(() => { const hmrContents = env.driveHmr('test.ts', 'Cmp'); expect(jsContents).toContain(`import * as i0 from "@angular/core";`); expect(jsContents).toContain(`import * as i1 from "./dep";`); + expect(jsContents).toContain('const id = "test.ts%40Cmp";'); expect(jsContents).toContain('function Cmp_HmrLoad(t) {'); expect(jsContents).toContain( - 'import(/* @vite-ignore */\nnew URL("./@ng/component?c=test.ts%40Cmp&t=" + encodeURIComponent(t), import.meta.url).href)', + 'import(/* @vite-ignore */\nnew URL("./@ng/component?c=" + id + "&t=" + encodeURIComponent(t), import.meta.url).href)', ); expect(jsContents).toContain( ').then(m => m.default && i0.ɵɵreplaceMetadata(Cmp, m.default, [i0, i1], ' + - '[DepModule, Component]));', + '[DepModule, Component], import.meta, id));', ); expect(jsContents).toContain('Cmp_HmrLoad(Date.now());'); expect(jsContents).toContain( 'import.meta.hot && import.meta.hot.on("angular:component-update", ' + - 'd => d.id === "test.ts%40Cmp" && Cmp_HmrLoad(d.timestamp)', + 'd => d.id === id && Cmp_HmrLoad(d.timestamp)', ); expect(hmrContents).toContain( @@ -340,7 +342,9 @@ runInEachFileSystem(() => { expect(jsContents).toContain('const Cmp_Defer_1_DepsFn = () => [Dep];'); expect(jsContents).toContain('function Cmp_Defer_0_Template(rf, ctx) { if (rf & 1) {'); expect(jsContents).toContain('i0.ɵɵdefer(1, 0, Cmp_Defer_1_DepsFn);'); - expect(jsContents).toContain('ɵɵreplaceMetadata(Cmp, m.default, [i0], [Dep]));'); + expect(jsContents).toContain( + 'ɵɵreplaceMetadata(Cmp, m.default, [i0], [Dep], import.meta, id));', + ); expect(jsContents).not.toContain('setClassMetadata'); expect(hmrContents).toContain( @@ -422,7 +426,9 @@ runInEachFileSystem(() => { const jsContents = env.getContents('test.js'); const hmrContents = env.driveHmr('test.ts', 'Cmp'); expect(jsContents).toContain('dependencies: [Cmp]'); - expect(jsContents).toContain('ɵɵreplaceMetadata(Cmp, m.default, [i0], [Component]));'); + expect(jsContents).toContain( + 'ɵɵreplaceMetadata(Cmp, m.default, [i0], [Component], import.meta, id));', + ); expect(hmrContents).toContain( 'export default function Cmp_UpdateMetadata(Cmp, ɵɵnamespaces, Component) {', ); @@ -445,7 +451,9 @@ runInEachFileSystem(() => { const jsContents = env.getContents('test.js'); const hmrContents = env.driveHmr('test.ts', 'Cmp'); expect(jsContents).not.toContain('dependencies'); - expect(jsContents).toContain('ɵɵreplaceMetadata(Cmp, m.default, [i0], [Component]));'); + expect(jsContents).toContain( + 'ɵɵreplaceMetadata(Cmp, m.default, [i0], [Component], import.meta, id));', + ); expect(hmrContents).toContain( 'export default function Cmp_UpdateMetadata(Cmp, ɵɵnamespaces, Component) {', ); @@ -471,7 +479,7 @@ runInEachFileSystem(() => { const hmrContents = env.driveHmr('test.ts', 'Cmp'); expect(jsContents).toContain( - 'ɵɵreplaceMetadata(Cmp, m.default, [i0], [providers, Component]));', + 'ɵɵreplaceMetadata(Cmp, m.default, [i0], [providers, Component], import.meta, id));', ); expect(hmrContents).toContain( 'export default function Cmp_UpdateMetadata(Cmp, ɵɵnamespaces, providers, Component) {', @@ -508,7 +516,7 @@ runInEachFileSystem(() => { const hmrContents = env.driveHmr('test.ts', 'Cmp'); expect(jsContents).toContain( - 'ɵɵreplaceMetadata(Cmp, m.default, [i0], [token, value, Component]));', + 'ɵɵreplaceMetadata(Cmp, m.default, [i0], [token, value, Component], import.meta, id));', ); expect(hmrContents).toContain( 'export default function Cmp_UpdateMetadata(Cmp, ɵɵnamespaces, token, value, Component) {', @@ -542,7 +550,7 @@ runInEachFileSystem(() => { const hmrContents = env.driveHmr('test.ts', 'Cmp'); expect(jsContents).toContain( - 'ɵɵreplaceMetadata(Cmp, m.default, [i0], [token, value, Component]));', + 'ɵɵreplaceMetadata(Cmp, m.default, [i0], [token, value, Component], import.meta, id));', ); expect(hmrContents).toContain( 'export default function Cmp_UpdateMetadata(Cmp, ɵɵnamespaces, token, value, Component) {', @@ -574,7 +582,7 @@ runInEachFileSystem(() => { const hmrContents = env.driveHmr('test.ts', 'Cmp'); expect(jsContents).toContain( - 'ɵɵreplaceMetadata(Cmp, m.default, [i0], [condition, providersA, providersB, Component]));', + 'ɵɵreplaceMetadata(Cmp, m.default, [i0], [condition, providersA, providersB, Component], import.meta, id));', ); expect(hmrContents).toContain( 'export default function Cmp_UpdateMetadata(Cmp, ɵɵnamespaces, condition, providersA, providersB, Component) {', @@ -608,7 +616,7 @@ runInEachFileSystem(() => { const jsContents = env.getContents('test.js'); const hmrContents = env.driveHmr('test.ts', 'Cmp'); expect(jsContents).toContain( - 'ɵɵreplaceMetadata(Cmp, m.default, [i0], [token, value, otherValue, Component]));', + 'ɵɵreplaceMetadata(Cmp, m.default, [i0], [token, value, otherValue, Component], import.meta, id));', ); expect(jsContents).toContain('useFactory: () => [(value), ((((otherValue))))]'); expect(hmrContents).toContain( @@ -646,7 +654,7 @@ runInEachFileSystem(() => { const hmrContents = env.driveHmr('test.ts', 'Cmp'); expect(jsContents).toContain( - 'ɵɵreplaceMetadata(Cmp, m.default, [i0], [token, value, Optional, dep, Component]));', + 'ɵɵreplaceMetadata(Cmp, m.default, [i0], [token, value, Optional, dep, Component], import.meta, id));', ); expect(hmrContents).toContain( 'export default function Cmp_UpdateMetadata(Cmp, ɵɵnamespaces, token, value, Optional, dep, Component) {', @@ -697,7 +705,9 @@ runInEachFileSystem(() => { const hmrContents = env.driveHmr('test.ts', 'Cmp'); expect(jsContents).toContain('dependencies: [Dep]'); - expect(jsContents).toContain('ɵɵreplaceMetadata(Cmp, m.default, [i0], [Dep]));'); + expect(jsContents).toContain( + 'ɵɵreplaceMetadata(Cmp, m.default, [i0], [Dep], import.meta, id));', + ); expect(hmrContents).toContain('function Cmp_UpdateMetadata(Cmp, ɵɵnamespaces, Dep) {'); }); @@ -741,7 +751,9 @@ runInEachFileSystem(() => { const hmrContents = env.driveHmr('test.ts', 'Cmp'); expect(jsContents).toContain('dependencies: [DepModule, i1.Dep]'); - expect(jsContents).toContain('ɵɵreplaceMetadata(Cmp, m.default, [i0, i1], [DepModule]));'); + expect(jsContents).toContain( + 'ɵɵreplaceMetadata(Cmp, m.default, [i0, i1], [DepModule], import.meta, id));', + ); expect(hmrContents).toContain('function Cmp_UpdateMetadata(Cmp, ɵɵnamespaces, DepModule) {'); }); @@ -797,7 +809,9 @@ runInEachFileSystem(() => { const hmrContents = env.driveHmr('test.ts', 'Cmp'); expect(jsContents).toContain('dependencies: [i1.Dep]'); - expect(jsContents).toContain('ɵɵreplaceMetadata(Cmp, m.default, [i0, i1], []));'); + expect(jsContents).toContain( + 'ɵɵreplaceMetadata(Cmp, m.default, [i0, i1], [], import.meta, id));', + ); expect(hmrContents).toContain('function Cmp_UpdateMetadata(Cmp, ɵɵnamespaces) {'); }); @@ -834,7 +848,7 @@ runInEachFileSystem(() => { const jsContents = env.getContents('test.js'); const hmrContents = env.driveHmr('test.ts', 'Cmp'); expect(jsContents).toContain( - 'ɵɵreplaceMetadata(Cmp, m.default, [i0], [token, { one: 0, two: "2", three: 3 }, Component]));', + 'ɵɵreplaceMetadata(Cmp, m.default, [i0], [token, { one: 0, two: "2", three: 3 }, Component], import.meta, id));', ); expect(hmrContents).toContain( 'export default function Cmp_UpdateMetadata(Cmp, ɵɵnamespaces, token, Foo, Component) {', @@ -881,7 +895,7 @@ runInEachFileSystem(() => { const jsContents = env.getContents('test.js'); const hmrContents = env.driveHmr('test.ts', 'Cmp'); expect(jsContents).toContain( - 'ɵɵreplaceMetadata(Cmp, m.default, [i0], [token, { one: 0, two: "2", three: 3 }, Component]));', + 'ɵɵreplaceMetadata(Cmp, m.default, [i0], [token, { one: 0, two: "2", three: 3 }, Component], import.meta, id));', ); expect(hmrContents).toContain( 'export default function Cmp_UpdateMetadata(Cmp, ɵɵnamespaces, token, Foo, Component) {', diff --git a/packages/compiler/src/render3/r3_hmr_compiler.ts b/packages/compiler/src/render3/r3_hmr_compiler.ts index 1f6e7e2279e5..db916e07915e 100644 --- a/packages/compiler/src/render3/r3_hmr_compiler.ts +++ b/packages/compiler/src/render3/r3_hmr_compiler.ts @@ -53,11 +53,10 @@ export interface R3HmrNamespaceDependency { * @param meta HMR metadata extracted from the class. */ export function compileHmrInitializer(meta: R3HmrMetadata): o.Expression { - const id = encodeURIComponent(`${meta.filePath}@${meta.className}`); - const urlPartial = `./@ng/component?c=${id}&t=`; const moduleName = 'm'; const dataName = 'd'; const timestampName = 't'; + const idName = 'id'; const importCallbackName = `${meta.className}_HmrLoad`; const namespaces = meta.namespaceDependencies.map((dep) => { return new o.ExternalExpr({moduleName: dep.moduleName, name: null}); @@ -66,7 +65,7 @@ export function compileHmrInitializer(meta: R3HmrMetadata): o.Expression { // m.default const defaultRead = o.variable(moduleName).prop('default'); - // ɵɵreplaceMetadata(Comp, m.default, [...namespaces], [...locals]); + // ɵɵreplaceMetadata(Comp, m.default, [...namespaces], [...locals], import.meta, id); const replaceCall = o .importExpr(R3.replaceMetadata) .callFn([ @@ -74,14 +73,18 @@ export function compileHmrInitializer(meta: R3HmrMetadata): o.Expression { defaultRead, o.literalArr(namespaces), o.literalArr(meta.localDependencies.map((l) => l.runtimeRepresentation)), + o.variable('import').prop('meta'), + o.variable(idName), ]); // (m) => m.default && ɵɵreplaceMetadata(...) const replaceCallback = o.arrowFn([new o.FnParam(moduleName)], defaultRead.and(replaceCall)); - // '' + encodeURIComponent(t) + // '?c=' + id + '&t=' + encodeURIComponent(t) const urlValue = o - .literal(urlPartial) + .literal(`./@ng/component?c=`) + .plus(o.variable(idName)) + .plus(o.literal('&t=')) .plus(o.variable('encodeURIComponent').callFn([o.variable(timestampName)])); // import.meta.url @@ -109,13 +112,13 @@ export function compileHmrInitializer(meta: R3HmrMetadata): o.Expression { o.StmtModifier.Final, ); - // (d) => d.id === && Cmp_HmrLoad(d.timestamp) + // (d) => d.id === id && Cmp_HmrLoad(d.timestamp) const updateCallback = o.arrowFn( [new o.FnParam(dataName)], o .variable(dataName) .prop('id') - .identical(o.literal(id)) + .identical(o.variable(idName)) .and(o.variable(importCallbackName).callFn([o.variable(dataName).prop('timestamp')])), ); @@ -139,6 +142,13 @@ export function compileHmrInitializer(meta: R3HmrMetadata): o.Expression { .arrowFn( [], [ + // const id = ; + new o.DeclareVarStmt( + idName, + o.literal(encodeURIComponent(`${meta.filePath}@${meta.className}`)), + null, + o.StmtModifier.Final, + ), // function Cmp_HmrLoad() {...}. importCallback, // ngDevMode && Cmp_HmrLoad(Date.now()); diff --git a/packages/core/src/render3/hmr.ts b/packages/core/src/render3/hmr.ts index bd3641ba52c5..c559885263ee 100644 --- a/packages/core/src/render3/hmr.ts +++ b/packages/core/src/render3/hmr.ts @@ -51,6 +51,8 @@ import {NG_COMP_DEF} from './fields'; * @param applyMetadata Callback that will apply a new set of metadata on the `type` when invoked. * @param environment Syntehtic namespace imports that need to be passed along to the callback. * @param locals Local symbols from the source location that have to be exposed to the callback. + * @param id ID to the class being replaced. **Not** the same as the component definition ID. + * Optional since the ID might not be available internally. * @codeGenApi */ export function ɵɵreplaceMetadata( @@ -58,6 +60,7 @@ export function ɵɵreplaceMetadata( applyMetadata: (...args: [Type, unknown[], ...unknown[]]) => void, namespaces: unknown[], locals: unknown[], + id: string | null = null, ) { ngDevMode && assertComponentDef(type); const currentDef = getComponentDef(type)!; diff --git a/packages/core/test/acceptance/hmr_spec.ts b/packages/core/test/acceptance/hmr_spec.ts index de012c7e8875..b964e7812690 100644 --- a/packages/core/test/acceptance/hmr_spec.ts +++ b/packages/core/test/acceptance/hmr_spec.ts @@ -2157,6 +2157,7 @@ describe('hot module replacement', () => { }, [angularCoreEnv], [], + '', ); } From 5e5e7c937a7a54be12774698c6767a6b74cb3896 Mon Sep 17 00:00:00 2001 From: Kristiyan Kostadinov Date: Thu, 6 Feb 2025 13:52:20 -0800 Subject: [PATCH 2/2] fix(core): invalidate HMR component if replacement throws an error Integrates https://github.com/angular/angular-cli/pull/29510 which allows us to invalidate the data in the dev server for a component if a replacement threw an error. --- packages/core/src/render3/hmr.ts | 59 ++++++++++++++++++++--- packages/core/test/acceptance/hmr_spec.ts | 1 + 2 files changed, 52 insertions(+), 8 deletions(-) diff --git a/packages/core/src/render3/hmr.ts b/packages/core/src/render3/hmr.ts index c559885263ee..0ea140b35648 100644 --- a/packages/core/src/render3/hmr.ts +++ b/packages/core/src/render3/hmr.ts @@ -45,14 +45,23 @@ import {NgZone} from '../zone'; import {ViewEncapsulation} from '../metadata/view'; import {NG_COMP_DEF} from './fields'; +/** Represents `import.meta` plus some information that's not in the built-in types. */ +type ImportMetaExtended = ImportMeta & { + hot?: { + send?: (name: string, payload: unknown) => void; + }; +}; + /** * Replaces the metadata of a component type and re-renders all live instances of the component. * @param type Class whose metadata will be replaced. * @param applyMetadata Callback that will apply a new set of metadata on the `type` when invoked. * @param environment Syntehtic namespace imports that need to be passed along to the callback. * @param locals Local symbols from the source location that have to be exposed to the callback. + * @param importMeta `import.meta` from the call site of the replacement function. Optional since + * it isn't used internally. * @param id ID to the class being replaced. **Not** the same as the component definition ID. - * Optional since the ID might not be available internally. + * Optional since the ID might not be available internally. * @codeGenApi */ export function ɵɵreplaceMetadata( @@ -60,6 +69,7 @@ export function ɵɵreplaceMetadata( applyMetadata: (...args: [Type, unknown[], ...unknown[]]) => void, namespaces: unknown[], locals: unknown[], + importMeta: ImportMetaExtended | null = null, id: string | null = null, ) { ngDevMode && assertComponentDef(type); @@ -87,7 +97,7 @@ export function ɵɵreplaceMetadata( // Note: we have the additional check, because `IsRoot` can also indicate // a component created through something like `createComponent`. if (isRootView(root) && root[PARENT] === null) { - recreateMatchingLViews(newDef, oldDef, root); + recreateMatchingLViews(importMeta, id, newDef, oldDef, root); } } } @@ -132,10 +142,14 @@ function mergeWithExistingDefinition( /** * Finds all LViews matching a specific component definition and recreates them. + * @param importMeta `import.meta` information. + * @param id HMR ID of the component. * @param oldDef Component definition to search for. * @param rootLView View from which to start the search. */ function recreateMatchingLViews( + importMeta: ImportMetaExtended | null, + id: string | null, newDef: ComponentDef, oldDef: ComponentDef, rootLView: LView, @@ -152,7 +166,7 @@ function recreateMatchingLViews( // produce false positives when using inheritance. if (tView === oldDef.tView) { ngDevMode && assertComponentDef(oldDef.type); - recreateLView(newDef, oldDef, rootLView); + recreateLView(importMeta, id, newDef, oldDef, rootLView); return; } @@ -162,14 +176,14 @@ function recreateMatchingLViews( if (isLContainer(current)) { // The host can be an LView if a component is injecting `ViewContainerRef`. if (isLView(current[HOST])) { - recreateMatchingLViews(newDef, oldDef, current[HOST]); + recreateMatchingLViews(importMeta, id, newDef, oldDef, current[HOST]); } for (let j = CONTAINER_HEADER_OFFSET; j < current.length; j++) { - recreateMatchingLViews(newDef, oldDef, current[j]); + recreateMatchingLViews(importMeta, id, newDef, oldDef, current[j]); } } else if (isLView(current)) { - recreateMatchingLViews(newDef, oldDef, current); + recreateMatchingLViews(importMeta, id, newDef, oldDef, current); } } } @@ -190,11 +204,15 @@ function clearRendererCache(factory: RendererFactory, def: ComponentDef /** * Recreates an LView in-place from a new component definition. + * @param importMeta `import.meta` information. + * @param id HMR ID for the component. * @param newDef Definition from which to recreate the view. * @param oldDef Previous component definition being swapped out. * @param lView View to be recreated. */ function recreateLView( + importMeta: ImportMetaExtended | null, + id: string | null, newDef: ComponentDef, oldDef: ComponentDef, lView: LView, @@ -272,9 +290,34 @@ function recreateLView( // The callback isn't guaranteed to be inside the Zone so we need to bring it in ourselves. if (zone === null) { - recreate(); + executeWithInvalidateFallback(importMeta, id, recreate); } else { - zone.run(recreate); + zone.run(() => executeWithInvalidateFallback(importMeta, id, recreate)); + } +} + +/** + * Runs an HMR-related function and falls back to + * invalidating the HMR data if it throws an error. + */ +function executeWithInvalidateFallback( + importMeta: ImportMetaExtended | null, + id: string | null, + callback: () => void, +) { + try { + callback(); + } catch (e) { + const errorMessage = (e as {message?: string}).message; + + // If we have all the necessary information and APIs to send off the invalidation + // request, send it before rethrowing so the dev server can decide what to do. + if (id !== null && errorMessage) { + importMeta?.hot?.send?.('angular:invalidate', {id, message: errorMessage, error: true}); + } + + // Throw the error in case the page doesn't get refreshed. + throw e; } } diff --git a/packages/core/test/acceptance/hmr_spec.ts b/packages/core/test/acceptance/hmr_spec.ts index b964e7812690..ce88026f1a5e 100644 --- a/packages/core/test/acceptance/hmr_spec.ts +++ b/packages/core/test/acceptance/hmr_spec.ts @@ -2157,6 +2157,7 @@ describe('hot module replacement', () => { }, [angularCoreEnv], [], + null, '', ); }