mirror of
https://github.com/angular/angular.git
synced 2026-09-14 13:54:52 +08:00
fix(devtools): safely handle foreign root elements in feature detection
Prevent Angular DevTools from crashing when elements matching [ng-version] in the DOM do not belong to the host application (such as third-party custom elements or browser extensions). ng.getComponent returns null for these elements, which previously caused ng.getDirectiveMetadata to throw.
(cherry picked from commit ef09d108f2)
This commit is contained in:
committed by
Kristiyan Kostadinov
parent
356adf7491
commit
a4a8573a75
+1
@@ -35,6 +35,7 @@ ts_test_library(
|
||||
":get-roots",
|
||||
"//:node_modules/@angular/core",
|
||||
"//:node_modules/jasmine",
|
||||
"//devtools/projects/ng-devtools-backend/src/lib/directive-forest:core-enums",
|
||||
],
|
||||
)
|
||||
|
||||
|
||||
+21
@@ -13,10 +13,12 @@ import {
|
||||
InjectionToken,
|
||||
} from '@angular/core';
|
||||
import {
|
||||
getDirectiveCdStrategy,
|
||||
getInjectorFromElementNode,
|
||||
getRootElements,
|
||||
serializeProviderRecord,
|
||||
} from './component-tree';
|
||||
import {ChangeDetectionStrategy, Framework} from '../core-enums';
|
||||
|
||||
type Ng = ɵExternalCoreGlobalUtils;
|
||||
const NG_VERSION = 'ng-version';
|
||||
@@ -183,4 +185,23 @@ describe('component-tree', () => {
|
||||
expect(result.token).toBe('InjectionToken (FOO)');
|
||||
});
|
||||
});
|
||||
|
||||
describe('getDirectiveCdStrategy', () => {
|
||||
it('returns change detection strategy when passed valid component', () => {
|
||||
const ng: Partial<Ng> = {
|
||||
getDirectiveMetadata: jasmine.createSpy('getDirectiveMetadata').and.returnValue({
|
||||
framework: Framework.Angular,
|
||||
changeDetection: ChangeDetectionStrategy.OnPush,
|
||||
} as any),
|
||||
};
|
||||
(globalThis as any).ng = ng;
|
||||
|
||||
const result = getDirectiveCdStrategy({
|
||||
instance: {},
|
||||
name: 'Foo',
|
||||
isElement: false,
|
||||
});
|
||||
expect(result).toBe('ng-on-push');
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
+5
-5
@@ -228,9 +228,9 @@ const enum DirectiveMetadataKey {
|
||||
// Gets directive metadata. For newer versions of Angular (v12+) it uses
|
||||
// the global `getDirectiveMetadata`. For prior versions of the framework
|
||||
// the method directly interacts with the directive/component definition.
|
||||
const getDirectiveMetadata = (dir: any): DirectiveMetadata => {
|
||||
const getMetadata = ngDebugClient().getDirectiveMetadata!;
|
||||
const metadata = getMetadata?.(dir);
|
||||
function getDirectiveMetadata(dir: any): DirectiveMetadata {
|
||||
const getMetadata = ngDebugClient().getDirectiveMetadata;
|
||||
const metadata = dir ? getMetadata?.(dir) : null;
|
||||
if (metadata) {
|
||||
const {framework} = metadata;
|
||||
switch (framework) {
|
||||
@@ -287,9 +287,9 @@ const getDirectiveMetadata = (dir: any): DirectiveMetadata => {
|
||||
encapsulation: safelyGrabMetadata(DirectiveMetadataKey.ENCAPSULATION),
|
||||
changeDetection: safelyGrabMetadata(DirectiveMetadataKey.CHANGE_DETECTION),
|
||||
};
|
||||
};
|
||||
}
|
||||
|
||||
export function getDirectiveCdStrategy(dir: any): ChangeDetection | undefined {
|
||||
export function getDirectiveCdStrategy(dir: ComponentInstanceType): ChangeDetection | undefined {
|
||||
const metadata = getDirectiveMetadata(dir.instance);
|
||||
|
||||
switch (metadata.framework) {
|
||||
|
||||
+34
@@ -114,6 +114,23 @@ describe('ng-debug-api', () => {
|
||||
|
||||
expect(ngDebugRoutesApiIsSupported()).toBeFalse();
|
||||
});
|
||||
|
||||
it('should ignore elements where getComponent returns null (e.g. extension elements)', () => {
|
||||
const foreignRoot = document.createElement('foreign-extension-app');
|
||||
foreignRoot.setAttribute('ng-version', '0.0.0-PLACEHOLDER');
|
||||
document.body.appendChild(foreignRoot);
|
||||
|
||||
try {
|
||||
const fakeNg = fakeNgGlobal(Framework.Angular);
|
||||
const originalGetComponent = fakeNg.getComponent!;
|
||||
fakeNg.getComponent = (el) => (el === foreignRoot ? null : originalGetComponent(el));
|
||||
(globalThis as any).ng = fakeNg;
|
||||
|
||||
expect(ngDebugProfilerApiIsSupported()).toBeTrue();
|
||||
} finally {
|
||||
foreignRoot.remove();
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe('ngDebugRoutesApiIsSupported', () => {
|
||||
@@ -191,5 +208,22 @@ describe('ng-debug-api', () => {
|
||||
(globalThis as any).ng = fakeNgGlobal(Framework.ACX);
|
||||
expect(ngDebugSignalPropertiesInspectionApiIsSupported()).toBeFalse();
|
||||
});
|
||||
|
||||
it('should ignore elements where getComponent returns null (e.g. extension elements)', () => {
|
||||
const foreignRoot = document.createElement('foreign-extension-app');
|
||||
foreignRoot.setAttribute('ng-version', '0.0.0-PLACEHOLDER');
|
||||
document.body.appendChild(foreignRoot);
|
||||
|
||||
try {
|
||||
const fakeNg = fakeNgGlobal(Framework.Angular);
|
||||
const originalGetComponent = fakeNg.getComponent!;
|
||||
fakeNg.getComponent = (el) => (el === foreignRoot ? null : originalGetComponent(el));
|
||||
(globalThis as any).ng = fakeNg;
|
||||
|
||||
expect(ngDebugSignalPropertiesInspectionApiIsSupported()).toBeTrue();
|
||||
} finally {
|
||||
foreignRoot.remove();
|
||||
}
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -58,7 +58,7 @@ export function ngDebugProfilerApiIsSupported(): boolean {
|
||||
// Temporary solution. Convert to an eligible API when available.
|
||||
// https://github.com/angular/angular/pull/60585#discussion_r2017047132
|
||||
// If there is a Wiz application, make Profiler API unavailable.
|
||||
const roots = getAppRoots();
|
||||
const roots = getAppRoots().filter((el) => ng.getComponent?.(el));
|
||||
return (
|
||||
!!roots.length &&
|
||||
!roots.some((el) => {
|
||||
@@ -98,7 +98,7 @@ export function ngDebugSignalPropertiesInspectionApiIsSupported(): boolean {
|
||||
const ng = ngDebugClient();
|
||||
|
||||
// If all apps are Angular, make the API available.
|
||||
const roots = getAppRoots();
|
||||
const roots = getAppRoots().filter((el) => ng.getComponent?.(el));
|
||||
return (
|
||||
!!roots.length &&
|
||||
roots.every((el) => {
|
||||
|
||||
Reference in New Issue
Block a user