diff --git a/devtools/projects/ng-devtools-backend/src/lib/BUILD.bazel b/devtools/projects/ng-devtools-backend/src/lib/BUILD.bazel index 1d733c553ac..56f7c47e55b 100644 --- a/devtools/projects/ng-devtools-backend/src/lib/BUILD.bazel +++ b/devtools/projects/ng-devtools-backend/src/lib/BUILD.bazel @@ -55,6 +55,29 @@ ts_library( ], ) +ts_library( + name = "property_mutation", + srcs = ["property-mutation.ts"], + deps = [ + ":utils", + "//packages/core", + ], +) + +ts_test_library( + name = "property_mutation_test_lib", + srcs = ["property-mutation.spec.ts"], + deps = [ + ":property_mutation", + "//packages/core", + ], +) + +karma_web_test_suite( + name = "property_mutation_test", + deps = [":property_mutation_test_lib"], +) + karma_web_test_suite( name = "router_tree_test", deps = [ @@ -156,6 +179,7 @@ ts_library( srcs = ["component-tree.ts"], deps = [ ":interfaces", + ":property_mutation", ":utils", "//devtools/projects/ng-devtools-backend/src/lib/directive-forest", "//devtools/projects/ng-devtools-backend/src/lib/ng-debug-api", diff --git a/devtools/projects/ng-devtools-backend/src/lib/component-tree.ts b/devtools/projects/ng-devtools-backend/src/lib/component-tree.ts index f89a42b9739..acda1fb7928 100644 --- a/devtools/projects/ng-devtools-backend/src/lib/component-tree.ts +++ b/devtools/projects/ng-devtools-backend/src/lib/component-tree.ts @@ -5,39 +5,6 @@ * Use of this source code is governed by an MIT-style license that can be * found in the LICENSE file at https://angular.dev/license */ -import { - ComponentExplorerViewQuery, - DirectiveMetadata, - DirectivesProperties, - ElementPosition, - PropertyQueryTypes, - SerializedInjectedService, - SerializedInjector, - SerializedProviderRecord, - UpdatedStateData, -} from 'protocol'; - -import {buildDirectiveTree, getLViewFromDirectiveOrElementInstance} from './directive-forest/index'; -import { - ngDebugApiIsSupported, - ngDebugClient, - ngDebugDependencyInjectionApiIsSupported, -} from './ng-debug-api/ng-debug-api'; -import { - deeplySerializeSelectedProperties, - serializeDirectiveState, -} from './state-serializer/state-serializer'; - -// Need to be kept in sync with Angular framework -// We can't directly import it from framework now -// because this also pulls up the security policies -// for Trusted Types, which we reinstantiate. -enum ChangeDetectionStrategy { - OnPush = 0, - Default = 1, -} - -import {ComponentTreeNode, DirectiveInstanceType, ComponentInstanceType} from './interfaces'; import type { ClassProvider, @@ -51,7 +18,38 @@ import type { ɵComponentDebugMetadata as ComponentDebugMetadata, ɵProviderRecord as ProviderRecord, } from '@angular/core'; -import {isSignal} from './utils'; +import { + ComponentExplorerViewQuery, + DirectiveMetadata, + DirectivesProperties, + ElementPosition, + PropertyQueryTypes, + SerializedInjectedService, + SerializedInjector, + SerializedProviderRecord, + UpdatedStateData, +} from 'protocol'; +import {buildDirectiveTree, getLViewFromDirectiveOrElementInstance} from './directive-forest/index'; +import { + ngDebugApiIsSupported, + ngDebugClient, + ngDebugDependencyInjectionApiIsSupported, +} from './ng-debug-api/ng-debug-api'; +import { + deeplySerializeSelectedProperties, + serializeDirectiveState, +} from './state-serializer/state-serializer'; +import {mutateNestedProp} from './property-mutation'; +import {ComponentTreeNode, DirectiveInstanceType, ComponentInstanceType} from './interfaces'; + +// Need to be kept in sync with Angular framework +// We can't directly import it from framework now +// because this also pulls up the security policies +// for Trusted Types, which we reinstantiate. +enum ChangeDetectionStrategy { + OnPush = 0, + Default = 1, +} export const injectorToId = new WeakMap(); export const nodeInjectorToResolutionPath = new WeakMap(); @@ -581,7 +579,7 @@ export const updateState = (updatedStateData: UpdatedStateData): void => { } if (updatedStateData.directiveId.directive !== undefined) { const directive = node.directives[updatedStateData.directiveId.directive].instance; - mutateComponentOrDirective(updatedStateData, directive); + mutateNestedProp(directive, updatedStateData.keyPath, updatedStateData.newValue); if (ngDebugApiIsSupported(ng, 'getOwningComponent')) { ng.applyChanges?.(ng.getOwningComponent(directive)!); } @@ -589,39 +587,12 @@ export const updateState = (updatedStateData: UpdatedStateData): void => { } if (node.component) { const comp = node.component.instance; - mutateComponentOrDirective(updatedStateData, comp); + mutateNestedProp(comp, updatedStateData.keyPath, updatedStateData.newValue); ng.applyChanges?.(comp); return; } }; -const mutateComponentOrDirective = (updatedStateData: UpdatedStateData, compOrDirective: any) => { - const valueKey = updatedStateData.keyPath.pop(); - if (valueKey === undefined) { - return; - } - - let parentObjectOfValueToUpdate = compOrDirective; - updatedStateData.keyPath.forEach((key) => { - parentObjectOfValueToUpdate = parentObjectOfValueToUpdate[key]; - }); - - if (isSignal(parentObjectOfValueToUpdate)) { - // we don't support updating nested objects in signals yet - return; - } - - // When we try to set a property which only has a getter - // the line below could throw an error. - try { - if (isSignal(parentObjectOfValueToUpdate[valueKey])) { - parentObjectOfValueToUpdate[valueKey].set(updatedStateData.newValue); - } else { - parentObjectOfValueToUpdate[valueKey] = updatedStateData.newValue; - } - } catch {} -}; - export function serializeResolutionPath(resolutionPath: Injector[]): SerializedInjector[] { const serializedResolutionPath: SerializedInjector[] = []; diff --git a/devtools/projects/ng-devtools-backend/src/lib/property-mutation.spec.ts b/devtools/projects/ng-devtools-backend/src/lib/property-mutation.spec.ts new file mode 100644 index 00000000000..ada4a93cb20 --- /dev/null +++ b/devtools/projects/ng-devtools-backend/src/lib/property-mutation.spec.ts @@ -0,0 +1,348 @@ +/** + * @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.dev/license + */ + +import {computed, signal} from '@angular/core'; +import {mutateNestedProp} from './property-mutation'; + +describe('property-mutation', () => { + describe('mutateNestedProp', () => { + it('throws when no keys are given', () => { + expect(() => mutateNestedProp({}, [], 1)).toThrow(); + }); + + describe('without signals', () => { + it('mutates objects', () => { + const obj = {foo: 1}; + + mutateNestedProp(obj, ['foo'], 2); + + expect(obj.foo).toBe(2); + }); + + it('mutates arrays', () => { + const obj = [1]; + + mutateNestedProp(obj, ['0'], 2); + + expect(obj.length).toBe(1); + expect(obj[0]).toBe(2); + }); + + it('mutates nested objects', () => { + const obj = {foo: {bar: 1}}; + + mutateNestedProp(obj, ['foo', 'bar'], 2); + + expect(obj.foo.bar).toBe(2); + }); + + it('mutates nested arrays', () => { + const obj = [[1]]; + + mutateNestedProp(obj, ['0', '0'], 2); + + expect(obj[0][0]).toBe(2); + }); + + it("throws an error when mutating outside an array's bounds", () => { + const obj = {foo: [1]}; + + expect(() => mutateNestedProp(obj, ['foo', '1'], 2)).toThrowError(/index 1.*length 1/); + }); + + it('mutates non-primitive objects', () => { + class MyFoo { + bar = 1; + } + + const obj = {foo: new MyFoo()}; + + mutateNestedProp(obj, ['foo', 'bar'], 2); + + expect(obj.foo.bar).toBe(2); + }); + + it('throws an error when mutating a non-existent property', () => { + const obj = { + foo: { + bar: 1, + }, + }; + + expect(() => mutateNestedProp(obj, ['foo', 'baz'], 2)).toThrowError( + /Property `baz` is not defined/, + ); + }); + + it('throws on mutating a getter property', () => { + const obj = { + foo: { + get bar() { + return 1; + }, + }, + }; + + expect(() => mutateNestedProp(obj, ['foo', 'bar'], 2)).toThrowError( + /Cannot mutate getter property: bar/, + ); + }); + + it('mutates an object from a getter property', () => { + const bar = {baz: 1}; + const obj = { + foo: { + get bar() { + return bar; + }, + }, + }; + + mutateNestedProp(obj, ['foo', 'bar', 'baz'], 2); + + expect(obj.foo.bar.baz).toBe(2); + }); + + it('mutates a setter property', () => { + let bar = 1; + const obj = { + foo: { + get bar() { + return bar; + }, + + set bar(value: number) { + bar = value; + }, + }, + }; + + mutateNestedProp(obj, ['foo', 'bar'], 2); + + expect(obj.foo.bar).toBe(2); + }); + }); + + describe('with signals', () => { + it('updates when the last value is a signal', () => { + const obj = {foo: signal(1)}; + + mutateNestedProp(obj, ['foo'], 2); + + expect(obj.foo()).toBe(2); + }); + + it('immutably updates when the last receiver is a signal', () => { + const obj = {foo: signal({bar: 1})}; + const originalSignalValue = obj.foo(); + + mutateNestedProp(obj, ['foo', 'bar'], 2); + + expect(obj.foo()).not.toBe(originalSignalValue); // Immutable update. + expect(obj.foo()).toEqual({bar: 2}); + }); + + it('immutably updates objects inside of a signal and preserves other data', () => { + const obj = {foo: signal({bar: 1, baz: 2})}; + + mutateNestedProp(obj, ['foo', 'bar'], 3); + + expect(obj.foo()).toEqual({bar: 3, baz: 2}); + }); + + it('immutably updates arrays inside of a signal', () => { + const obj = {foo: signal([1])}; + const originalSignalValue = obj.foo(); + + mutateNestedProp(obj, ['foo', '0'], 2); + + expect(obj.foo()).not.toBe(originalSignalValue); + expect(obj.foo()).toEqual([2]); + }); + + it("throws an error when mutating outside an array's bounds", () => { + const obj = {foo: signal([1])}; + + expect(() => mutateNestedProp(obj, ['foo', '1'], 2)).toThrowError(/index 1.*length 1/); + }); + + it('immutably updates arrays inside of a signal and preserves other data', () => { + const obj = {foo: signal([1, 2, 3, 4, 5])}; + + mutateNestedProp(obj, ['foo', '2'], 6); + + expect(obj.foo()).toEqual([1, 2, 6, 4, 5]); + }); + + it('immutably updates nested value inside a signal', () => { + const obj = { + foo: signal({ + bar: { + baz: 1, + }, + }), + }; + const originalSignalValue = obj.foo(); + + mutateNestedProp(obj, ['foo', 'bar', 'baz'], 2); + + expect(obj.foo()).not.toBe(originalSignalValue); // Do not mutate original object. + expect(obj.foo()).toEqual({bar: {baz: 2}}); + }); + + it('throws an error when mutating a non-existent property', () => { + const obj = { + foo: signal({ + bar: 1, + }), + }; + + expect(() => mutateNestedProp(obj, ['foo', 'baz'], 2)).toThrowError( + /Property `baz` is not defined/, + ); + }); + + it('throws on non-primitive objects', () => { + class MyFoo { + bar = 1; + } + + const obj = {foo: signal(new MyFoo())}; + + expect(() => mutateNestedProp(obj, ['foo', 'bar'], 2)).toThrowError( + /Cannot immutably update type/, + ); + }); + + it('throws on objects with getters', () => { + const obj = { + foo: signal({ + get bar() { + return 1; + }, + }), + }; + + expect(() => mutateNestedProp(obj, ['foo', 'bar'], 2)).toThrowError( + /Cannot immutably update object/, + ); + }); + + it('throws on objects with setters', () => { + const obj = { + foo: signal({ + set bar(_value: number) {}, + }), + }; + + expect(() => mutateNestedProp(obj, ['foo', 'bar'], 1)).toThrowError( + /Cannot immutably update object/, + ); + }); + + it('throws on objects with sibling getters or setters', () => { + const obj = { + foo: signal({ + bar: 1, + get baz() { + return 2; + }, + }), + }; + + expect(() => mutateNestedProp(obj, ['foo', 'bar'], 2)).toThrowError( + /Cannot immutably update object/, + ); + }); + + it('throws on objects with inherited getters or setters', () => { + const proto = { + bar: 1, + get baz() { + return 2; + }, + }; + + const obj = { + foo: signal(Object.create(proto)), + }; + + expect(() => mutateNestedProp(obj, ['foo', 'bar'], 2)).toThrowError( + /Cannot immutably update object/, + ); + }); + + it('immutably updates objects with unrelated getters or setters', () => { + const obj = { + foo: signal({ + bar: 1, + + // Safe: `baz` will not be immutably reconstructed when `bar` changes. + baz: { + get property() { + return 2; + }, + + set property(_value: number) {}, + }, + }), + }; + + const originalBaz = obj.foo().baz; + + mutateNestedProp(obj, ['foo', 'bar'], 3); + + expect(obj.foo().bar).toBe(3); + expect(obj.foo().baz).toBe(originalBaz); + }); + + it('throws on readonly signals', () => { + const obj = { + foo: { + bar: computed(() => ({baz: 1})), + }, + }; + + expect(() => mutateNestedProp(obj, ['foo', 'bar', 'baz'], 2)).toThrowError( + 'Cannot mutate a readonly signal at `foo.bar`.', + ); + }); + + it('throws on nested signals', () => { + const obj = { + foo: signal({ + bar: signal({ + baz: 1, + }), + }), + }; + + expect(() => mutateNestedProp(obj, ['foo', 'bar', 'baz'], 2)).toThrowError( + 'Cannot mutate nested signals.', + ); + }); + + it('immutable updates objects with unrelated nested signals', () => { + const obj = { + foo: signal({ + bar: { + baz: signal(1), + hello: 2, + }, + }), + }; + const originalFoo = obj.foo(); + + mutateNestedProp(obj, ['foo', 'bar', 'hello'], 3); + + expect(obj.foo().bar.hello).toBe(3); + expect(obj.foo().bar.baz).toBe(originalFoo.bar.baz); // Nested signal should be the same. + }); + }); + }); +}); diff --git a/devtools/projects/ng-devtools-backend/src/lib/property-mutation.ts b/devtools/projects/ng-devtools-backend/src/lib/property-mutation.ts new file mode 100644 index 00000000000..be991938ae3 --- /dev/null +++ b/devtools/projects/ng-devtools-backend/src/lib/property-mutation.ts @@ -0,0 +1,221 @@ +/** + * @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.dev/license + */ + +import type {Signal, WritableSignal} from '@angular/core'; +import {isSignal} from './utils'; + +/** Represents a property access operation. */ +interface PropertyAccess { + /** + * The receiver of the property access (`foo` in `foo.bar`). + * If the receiver was a signal, this is not the signal function, but instead its + * underlying data. + */ + receiver: any; + + /** The key used in the property access (`'bar'` in `foo.bar`). */ + key: string; + + /** + * The value of the property access (result of `foo.bar`). + * If the value is a signal, this is still the signal *function*, not its + * underlying data. + */ + value: any; +} + +/** + * Mutates the nested property under the provided object and assigns the new value. + * Supports both plain objects/arrays as well deep mutations within a signal. + * + * @param obj The object to assign to. + * @param keyPath An array of strings representing nested properties to be + * assigned. + * @param newValue The value to assign. + * + * Example: + * + * ```typescript + * const obj = {foo: {bar: [1]}}; + * mutateNestedProp(obj, ['foo', 'bar', '0'], 2); + * obj.foo.bar[0]; // 2 + * ``` + * + * When no signals are present, this performs a basic property assignment. + * + * ```typescript + * mutateNestedProp(obj, ['foo', 'bar', '0'], 1); + * // Equivalent to: + * obj.foo.bar[0] = 1; + * ``` + * + * Signals are handled by performing an immutable copy of their contents. + * + * ```typescript + * mutateNestedProp(obj, ['foo', 'bar', '1'], 2); + * // If `foo` is a signal, equivalent to: + * obj.foo.set({ + * ...obj.foo(), + * bar: [ + * ...obj.foo().bar.slice(0, 1), + * 2, + * ...obj.foo().bar.slice(2), + * ], + * }); + * ``` + * + * The algorithm works by walking the full list of properties and looking for a + * signal function. If one is found, we apply an immutable update on its contents + * and call `.set` with the result. If no signals are found, we get the last + * receiver in the chain and assign the value directly + * (`receiver[prop] = newValue;`). + */ +export function mutateNestedProp(obj: any, keyPath: string[], newValue: unknown): void { + if (keyPath.length === 0) throw new Error('At least one key is required.'); + + const nestedProps = Array.from(getNestedProps(obj, keyPath)); + + // Check for nested signals. + const signalCount = nestedProps.filter((prop) => isSignal(prop.value)).length; + if (signalCount > 1) throw new Error('Cannot mutate nested signals.'); + + // Check for a single signal. + const signalIndex = nestedProps.findIndex((prop) => isSignal(prop.value)); + if (signalIndex !== -1) { + const sig = nestedProps[signalIndex]; + const props = nestedProps.slice(signalIndex + 1); + + if (!isWritableSignal(sig.value)) { + const propPath = nestedProps + .slice(0, signalIndex + 1) + .map((prop) => prop.key) + .join('.'); + throw new Error(`Cannot mutate a readonly signal at \`${propPath}\`.`); + } + + sig.value.set(immutableUpdate(props, newValue)); + return; + } + + // No signals in this key path, just assign to the last receiver. + // First check to ensure there this is not a getter. + const finalProp = nestedProps[nestedProps.length - 1]; + const descriptor = getInheritedPropertyDescriptor(finalProp.receiver, finalProp.key); + if (descriptor && descriptor.get && !descriptor.set) { + throw new Error(`Cannot mutate getter property: ${finalProp.key}`); + } + + finalProp.receiver[finalProp.key] = newValue; +} + +/** Walk the object properties and generate {@link PropertyAccess} objects. */ +function* getNestedProps( + receiver: any, + keyPath: ReadonlyArray, +): Generator { + const keys = Array.from(keyPath); + while (keys.length !== 0) { + const key = keys.shift()!; + + if (Array.isArray(receiver) && parseInt(key) >= receiver.length) { + throw new Error(`Cannot access index ${key} for array of length ${receiver.length}.`); + } + + if (!(key in receiver)) { + throw new Error(`Property \`${key}\` is not defined on the object.`); + } + + const value = receiver[key]; + yield { + receiver, + key, + value, + }; + + receiver = isSignal(value) ? value() : value; + } +} + +/** + * Apply an immutable update assigning `newValue` to the list of properties + * provided. Generates new objects and retains existing properties while + * overwriting the specific nested property given. + */ +function immutableUpdate(props: PropertyAccess[], newValue: unknown): any { + if (props.length === 0) return newValue; + const [prop, ...remainingProps] = props; + + if (Array.isArray(prop.receiver)) { + const index = parseInt(prop.key); + + return [ + ...prop.receiver.slice(0, index), + immutableUpdate(remainingProps, newValue), + ...prop.receiver.slice(index + 1), + ]; + } else if (typeof prop.receiver === 'object') { + assertSafeToImmutablyUpdate(prop.receiver); + return { + ...prop.receiver, + [prop.key]: immutableUpdate(remainingProps, newValue), + }; + } else { + throw new Error(`Cannot immutably update type: ${prop.receiver.constructor.name}`); + } +} + +/** + * Gets the {@link PropertyDescriptor} for the given name, even if inherited from the + * prototype. This is effectively equivalent to + * {@link Object.getOwnPropertyDescriptor}, except it is not limited to "own" + * properties. + */ +function getInheritedPropertyDescriptor( + obj: {} | null, + prop: string, +): PropertyDescriptor | undefined { + if (obj === null) return undefined; + + const descriptor = Object.getOwnPropertyDescriptor(obj, prop); + if (descriptor) return descriptor; + + return getInheritedPropertyDescriptor(Object.getPrototypeOf(obj), prop); +} + +/** Assert that a given signal is writable. */ +function isWritableSignal(sig: Signal): sig is WritableSignal { + return 'set' in sig; +} + +/** + * Assert that the given object may be safely updated in an immutable fashion. + * + * Objects with user-defined prototypes may not be immutably updated because they + * would lose their prototypes. + * + * ```typescript + * ({...new MyFoo()}).doSomething(); // Doesn't work. + * ``` + */ +function assertSafeToImmutablyUpdate(obj: {}): void { + // `new MyClass()` case. + if (obj.constructor !== Object) { + throw new Error(`Cannot immutably update type: ${obj.constructor.name}`); + } + + // `{ get foo() { return 'foo'; } }` case. + // Need to check all inherited property descriptors, not just own descriptors. + for (const prop in obj) { + const descriptor = getInheritedPropertyDescriptor(obj, prop); + if (!descriptor) continue; + + if (descriptor.get || descriptor.set) { + throw new Error('Cannot immutably update object with getters or setters.'); + } + } +}