From 0c1ecb453c8b01447fb5a9f38fdfe9936a60abe5 Mon Sep 17 00:00:00 2001 From: Kristiyan Kostadinov Date: Wed, 29 Jan 2025 09:49:39 +0100 Subject: [PATCH] fix(elements): not setting initial value on signal-based input (#59773) Fixes that `createCustomElement` was incorrectly excluding signal-based inputs when setting the initial values. Fixes #59757. PR Close #59773 --- .../elements/src/create-custom-element.ts | 8 ++--- .../test/create-custom-element_spec.ts | 36 ++++++++++++++++++- 2 files changed, 39 insertions(+), 5 deletions(-) diff --git a/packages/elements/src/create-custom-element.ts b/packages/elements/src/create-custom-element.ts index 4d1864f2b2a..17dfd2a2722 100644 --- a/packages/elements/src/create-custom-element.ts +++ b/packages/elements/src/create-custom-element.ts @@ -156,13 +156,13 @@ export function createCustomElement

( // Re-apply pre-existing input values (set as properties on the element) through the // strategy. // TODO(alxhub): why are we doing this? this makes no sense. - inputs.forEach(({propName, transform, isSignal}) => { - if (!this.hasOwnProperty(propName) || isSignal) { - // No pre-existing value for `propName`, or a signal input. + inputs.forEach(({propName, transform}) => { + if (!this.hasOwnProperty(propName)) { + // No pre-existing value for `propName`. return; } - // Delete the property from the instance and re-apply it through the strategy. + // Delete the property from the DOM node and re-apply it through the strategy. const value = (this as any)[propName]; delete (this as any)[propName]; strategy.setInputValue(propName, value, transform); diff --git a/packages/elements/test/create-custom-element_spec.ts b/packages/elements/test/create-custom-element_spec.ts index 3bcd7f2af3f..d292579b62a 100644 --- a/packages/elements/test/create-custom-element_spec.ts +++ b/packages/elements/test/create-custom-element_spec.ts @@ -12,6 +12,7 @@ import { DoBootstrap, EventEmitter, Injector, + input, Input, NgModule, Output, @@ -31,6 +32,7 @@ interface WithFooBar { fooFoo: string; barBar: string; fooTransformed: unknown; + fooSignal: string | null; } describe('createCustomElement', () => { @@ -66,7 +68,12 @@ describe('createCustomElement', () => { }); it('should use a default strategy for converting component inputs', () => { - expect(NgElementCtor.observedAttributes).toEqual(['foo-foo', 'barbar', 'foo-transformed']); + expect(NgElementCtor.observedAttributes).toEqual([ + 'foo-foo', + 'barbar', + 'foo-transformed', + 'foo-signal', + ]); }); it('should send input values from attributes when connected', () => { @@ -74,12 +81,14 @@ describe('createCustomElement', () => { element.setAttribute('foo-foo', 'value-foo-foo'); element.setAttribute('barbar', 'value-barbar'); element.setAttribute('foo-transformed', 'truthy'); + element.setAttribute('foo-signal', 'value-signal'); element.connectedCallback(); expect(strategy.connectedElement).toBe(element); expect(strategy.getInputValue('fooFoo')).toBe('value-foo-foo'); expect(strategy.getInputValue('barBar')).toBe('value-barbar'); expect(strategy.getInputValue('fooTransformed')).toBe(true); + expect(strategy.getInputValue('fooSignal')).toBe('value-signal'); }); it('should work even if the constructor is not called (due to polyfill)', () => { @@ -95,12 +104,14 @@ describe('createCustomElement', () => { element.setAttribute('foo-foo', 'value-foo-foo'); element.setAttribute('barbar', 'value-barbar'); element.setAttribute('foo-transformed', 'truthy'); + element.setAttribute('foo-signal', 'value-signal'); element.connectedCallback(); expect(strategy.connectedElement).toBe(element); expect(strategy.getInputValue('fooFoo')).toBe('value-foo-foo'); expect(strategy.getInputValue('barBar')).toBe('value-barbar'); expect(strategy.getInputValue('fooTransformed')).toBe(true); + expect(strategy.getInputValue('fooSignal')).toBe('value-signal'); }); it('should listen to output events after connected', () => { @@ -174,10 +185,12 @@ describe('createCustomElement', () => { element.fooFoo = 'foo-foo-value'; element.barBar = 'barBar-value'; element.fooTransformed = 'truthy'; + element.fooSignal = 'value-signal'; expect(strategy.inputs.get('fooFoo')).toBe('foo-foo-value'); expect(strategy.inputs.get('barBar')).toBe('barBar-value'); expect(strategy.inputs.get('fooTransformed')).toBe(true); + expect(strategy.inputs.get('fooSignal')).toBe('value-signal'); }); it('should properly handle getting/setting properties on the element even if the constructor is not called', () => { @@ -191,10 +204,12 @@ describe('createCustomElement', () => { element.fooFoo = 'foo-foo-value'; element.barBar = 'barBar-value'; element.fooTransformed = 'truthy'; + element.fooSignal = 'value-signal'; expect(strategy.inputs.get('fooFoo')).toBe('foo-foo-value'); expect(strategy.inputs.get('barBar')).toBe('barBar-value'); expect(strategy.inputs.get('fooTransformed')).toBe(true); + expect(strategy.inputs.get('fooSignal')).toBe('value-signal'); }); it('should capture properties set before upgrading the element', () => { @@ -204,10 +219,12 @@ describe('createCustomElement', () => { fooFoo: 'foo-prop-value', barBar: 'bar-prop-value', fooTransformed: 'truthy' as unknown, + fooSignal: 'value-signal', }); expect(element.fooFoo).toBe('foo-prop-value'); expect(element.barBar).toBe('bar-prop-value'); expect(element.fooTransformed).toBe('truthy'); + expect(element.fooSignal).toBe('value-signal'); // Upgrade the element to a Custom Element and insert it into the DOM. customElements.define(selector, ElementCtor); @@ -215,10 +232,12 @@ describe('createCustomElement', () => { expect(element.fooFoo).toBe('foo-prop-value'); expect(element.barBar).toBe('bar-prop-value'); expect(element.fooTransformed).toBe(true); + expect(element.fooSignal).toBe('value-signal'); expect(strategy.inputs.get('fooFoo')).toBe('foo-prop-value'); expect(strategy.inputs.get('barBar')).toBe('bar-prop-value'); expect(strategy.inputs.get('fooTransformed')).toBe(true); + expect(strategy.inputs.get('fooSignal')).toBe('value-signal'); }); it('should capture properties set after upgrading the element but before inserting it into the DOM', () => { @@ -228,10 +247,12 @@ describe('createCustomElement', () => { fooFoo: 'foo-prop-value', barBar: 'bar-prop-value', fooTransformed: 'truthy' as unknown, + fooSignal: 'value-signal', }); expect(element.fooFoo).toBe('foo-prop-value'); expect(element.barBar).toBe('bar-prop-value'); expect(element.fooTransformed).toBe('truthy'); + expect(element.fooSignal).toBe('value-signal'); // Upgrade the element to a Custom Element (without inserting it into the DOM) and update a // property. @@ -239,19 +260,23 @@ describe('createCustomElement', () => { customElements.upgrade(element); element.barBar = 'bar-prop-value-2'; element.fooTransformed = ''; + element.fooSignal = 'value-signal-changed'; expect(element.fooFoo).toBe('foo-prop-value'); expect(element.barBar).toBe('bar-prop-value-2'); expect(element.fooTransformed).toBe(''); + expect(element.fooSignal).toBe('value-signal-changed'); // Insert the element into the DOM. testContainer.appendChild(element); expect(element.fooFoo).toBe('foo-prop-value'); expect(element.barBar).toBe('bar-prop-value-2'); expect(element.fooTransformed).toBe(false); + expect(element.fooSignal).toBe('value-signal-changed'); expect(strategy.inputs.get('fooFoo')).toBe('foo-prop-value'); expect(strategy.inputs.get('barBar')).toBe('bar-prop-value-2'); expect(strategy.inputs.get('fooTransformed')).toBe(false); + expect(strategy.inputs.get('fooSignal')).toBe('value-signal-changed'); }); it('should allow overwriting properties with attributes after upgrading the element but before inserting it into the DOM', () => { @@ -261,10 +286,12 @@ describe('createCustomElement', () => { fooFoo: 'foo-prop-value', barBar: 'bar-prop-value', fooTransformed: 'truthy' as unknown, + fooSignal: 'value-signal', }); expect(element.fooFoo).toBe('foo-prop-value'); expect(element.barBar).toBe('bar-prop-value'); expect(element.fooTransformed).toBe('truthy'); + expect(element.fooSignal).toBe('value-signal'); // Upgrade the element to a Custom Element (without inserting it into the DOM) and set an // attribute. @@ -275,16 +302,19 @@ describe('createCustomElement', () => { expect(element.fooFoo).toBe('foo-prop-value'); expect(element.barBar).toBe('bar-attr-value'); expect(element.fooTransformed).toBe(false); + expect(element.fooSignal).toBe('value-signal'); // Insert the element into the DOM. testContainer.appendChild(element); expect(element.fooFoo).toBe('foo-prop-value'); expect(element.barBar).toBe('bar-attr-value'); expect(element.fooTransformed).toBe(false); + expect(element.fooSignal).toBe('value-signal'); expect(strategy.inputs.get('fooFoo')).toBe('foo-prop-value'); expect(strategy.inputs.get('barBar')).toBe('bar-attr-value'); expect(strategy.inputs.get('fooTransformed')).toBe(false); + expect(strategy.inputs.get('fooSignal')).toBe('value-signal'); }); // Helpers @@ -313,6 +343,10 @@ describe('createCustomElement', () => { @Input('barbar') barBar!: string; @Input({transform: (value: unknown) => !!value}) fooTransformed!: boolean; + // This needs to apply the decorator and pass `isSignal`, because + // the compiler transform doesn't run against JIT tests. + @Input({isSignal: true} as Input) fooSignal = input(null); + @Output() bazBaz = new EventEmitter(); @Output('quxqux') quxQux = new EventEmitter(); }