diff --git a/packages/ts/react-signals/src/Signals.ts b/packages/ts/react-signals/src/Signals.ts index 759fb2d4a3..77636d40ff 100644 --- a/packages/ts/react-signals/src/Signals.ts +++ b/packages/ts/react-signals/src/Signals.ts @@ -1,4 +1,4 @@ -import { ValueSignal } from './ValueSignal.js'; +import { type OperationSubscription, ValueSignal } from './ValueSignal.js'; /** * A signal that holds a number value. The underlying @@ -23,4 +23,87 @@ import { ValueSignal } from './ValueSignal.js'; * ); * ``` */ -export class NumberSignal extends ValueSignal {} +export class NumberSignal extends ValueSignal { + /** + * Increments the value by the specified delta. The delta can be negative to + * decrease the value. If no delta is provided, the value increases by 1. + * + * This method differs from using the `++` or `+=` operators directly on the + * signal instance. It performs an atomic operation to prevent conflicts from + * concurrent changes, ensuring that other users' modifications are not + * accidentally overwritten. + * + * By default, if there are concurrent changes on the server, this operation + * will keep retrying until it succeeds. The returned `OperationSubscription` + * can be used to cancel the operation, if needed. + * To disable automatic retries, set `retryOnFailure` to `false`. Note that + * when `retryOnFailure` is `false`, the returned `OperationSubscription` will + * not be cancellable. + * + * @remarks + * **IMPORTANT:** please note that if the operation has already succeeded on + * the server, calling `cancel` will not prevent or undo the operation.The + * `cancel` method only stops further retry attempts if a previous attempt was + * rejected by the server. + * + * @param delta - The delta to increment the value by. The delta can be + * negative. Defaults to 1. + * @param retryOnFailure - Whether to retry the operation in case of a + * concurrent value change on the server or not. Defaults to `true`. + * + * @returns An object that can be used to cancel the operation. + */ + increment(delta: number = 1, retryOnFailure: boolean = true): OperationSubscription { + const noopCancelSubscription = { cancel: () => {} }; + if (delta === 0) { + return noopCancelSubscription; + } + if (!retryOnFailure) { + const expected = this.value; + this.replace(expected, expected + delta); + return noopCancelSubscription; + } + return this.update((value) => value + delta); + } + + /** + * Adds the provided delta to the value. The delta can be negative to perform + * a subtraction. + * This operation does not retry in case failure due to concurrent value + * changes from other clients have already applied to the shared value on the + * server. If you want the operation to be retried until it succeeds, use the + * overload with retry functionality. + * + * @param delta - The delta to add to the value. The delta can be negative to + * perform a subtraction. + * @see {@link NumberSignal.add|add(delta: number, retryOnFailure: boolean)} + */ + add(delta: number): void; + + /** + * Adds the provided delta to the value. The delta can be negative to perform + * a subtraction. + * If `true` passed to the `retryOnFailure` parameter, the operation is + * retried in case of failures due to concurrent value changes on the server + * until it succeeds. Note that the returned operation subscription can be + * used to cancel the operation. + * + * @param delta - The delta to add to the value. The delta can be negative. + * @param retryOnFailure - Whether to retry the operation in case of failures + * due to concurrent value changes on the server or not. + * @returns An operation subscription that can be canceled. + */ + add(delta: number, retryOnFailure: boolean): OperationSubscription; + + add(delta: number, retryOnFailure?: boolean): OperationSubscription | void { + if (delta !== 0) { + if (retryOnFailure) { + return this.increment(delta, retryOnFailure); + } + const expected = this.value; + this.replace(expected, expected + delta); + } + // eslint-disable-next-line no-useless-return, consistent-return + return; + } +} diff --git a/packages/ts/react-signals/test/Signals.spec.tsx b/packages/ts/react-signals/test/Signals.spec.tsx index 0cba7d88ed..90c85961b6 100644 --- a/packages/ts/react-signals/test/Signals.spec.tsx +++ b/packages/ts/react-signals/test/Signals.spec.tsx @@ -9,37 +9,32 @@ import type { StateEvent } from '../src/events.js'; import type { ServerConnectionConfig } from '../src/FullStackSignal.js'; import { effect } from '../src/index.js'; import { NumberSignal } from '../src/index.js'; -import { nextFrame } from './utils.js'; +import { createSubscriptionStub, nextFrame, subscribeToSignalViaEffect } from './utils.js'; use(sinonChai); use(chaiLike); describe('@vaadin/hilla-react-signals', () => { let config: ServerConnectionConfig; + let subscription: sinon.SinonStubbedInstance>>; + let client: sinon.SinonStubbedInstance; + + function simulateReceivingSnapshot(eventId: string, value: number): void { + const [onNextCallback] = subscription.onNext.firstCall.args; + onNextCallback({ id: eventId, type: 'snapshot', value }); + } + + function simulateReceivingReject(eventId: string): void { + const [onNextCallback] = subscription.onNext.firstCall.args; + // @ts-expect-error value is not used in reject events + onNextCallback({ id: eventId, type: 'reject', value: 0 }); + } beforeEach(() => { - const client = sinon.createStubInstance(ConnectClient); + client = sinon.createStubInstance(ConnectClient); client.call.resolves(); - - const subscription = sinon.spy>>({ - cancel() {}, - context() { - return this; - }, - onComplete() { - return this; - }, - onError() { - return this; - }, - onNext() { - return this; - }, - onDisconnect() { - return this; - }, - }); // Mock the subscribe method + subscription = createSubscriptionStub(); client.subscribe.returns(subscription); config = { client, endpoint: 'TestEndpoint', method: 'testMethod' }; }); @@ -83,5 +78,163 @@ describe('@vaadin/hilla-react-signals', () => { expect(results).to.be.like([undefined, 42, 43]); }); + + it('should send the correct values in replace events when increment is called', () => { + const numberSignal = new NumberSignal(42, config); + subscribeToSignalViaEffect(numberSignal); + + numberSignal.increment(); + const [, , params1] = client.call.firstCall.args; + expect(client.call).to.have.been.calledWithMatch('SignalsHandler', 'update', { + clientSignalId: numberSignal.id, + // @ts-expect-error params.event type has id property + event: { id: params1?.event.id, type: 'replace', value: 43, expected: 42 }, + }); + // @ts-expect-error params.event type has id property + simulateReceivingSnapshot(params1?.event.id, 43); + expect(numberSignal.value).to.equal(43); + + numberSignal.increment(2); + const [, , params2] = client.call.secondCall.args; + expect(client.call).to.have.been.calledWithMatch('SignalsHandler', 'update', { + clientSignalId: numberSignal.id, + // @ts-expect-error params.event type has id property + event: { id: params2?.event.id, type: 'replace', value: 45, expected: 43 }, + }); + // @ts-expect-error params.event type has id property + simulateReceivingSnapshot(params2?.event.id, 45); + expect(numberSignal.value).to.equal(45); + + numberSignal.increment(-5); + const [, , params3] = client.call.thirdCall.args; + expect(client.call).to.have.been.calledWithMatch('SignalsHandler', 'update', { + clientSignalId: numberSignal.id, + // @ts-expect-error params.event type has id property + event: { id: params3?.event.id, type: 'replace', value: 40, expected: 45 }, + }); + // @ts-expect-error params.event type has id property + simulateReceivingSnapshot(params3?.event.id, 40); + expect(numberSignal.value).to.equal(40); + }); + + it('should retry after receiving reject events when increment is called by default', () => { + const numberSignal = new NumberSignal(42, config); + subscribeToSignalViaEffect(numberSignal); + + const incrementSubscription = numberSignal.increment(); + const [, , params1] = client.call.firstCall.args; + expect(client.call).to.have.been.calledWithMatch('SignalsHandler', 'update', { + clientSignalId: numberSignal.id, + // @ts-expect-error params.event type has id property + event: { id: params1?.event.id, type: 'replace', value: 43, expected: 42 }, + }); + + // @ts-expect-error params.event type has id property + simulateReceivingReject(params1?.event.id); + const [, , params2] = client.call.secondCall.args; + expect(client.call).to.have.been.calledWithMatch('SignalsHandler', 'update', { + clientSignalId: numberSignal.id, + // @ts-expect-error params.event type has id property + event: { id: params2?.event.id, type: 'replace', value: 43, expected: 42 }, + }); + + setTimeout(() => incrementSubscription.cancel(), 500); + }); + + it('should not retry after receiving reject events when increment is called with true as retryOnFailure', () => { + const numberSignal = new NumberSignal(42, config); + subscribeToSignalViaEffect(numberSignal); + + numberSignal.increment(1, false); + expect(client.call).to.have.been.calledOnce; + const [, , params1] = client.call.firstCall.args; + + // @ts-expect-error params.event type has id property + simulateReceivingReject(params1?.event.id); + // Verify that failure has not triggered the retry: + expect(client.call).to.have.been.calledOnce; + }); + + it('should not send any event to the server when increment is called with zero as delta', () => { + const numberSignal = new NumberSignal(42, config); + subscribeToSignalViaEffect(numberSignal); + + numberSignal.increment(0); + expect(client.call).not.to.have.been.called; + }); + + it('should send the correct values in replace events when add is called', () => { + const numberSignal = new NumberSignal(42, config); + subscribeToSignalViaEffect(numberSignal); + + numberSignal.add(5); + const [, , params1] = client.call.firstCall.args; + expect(client.call).to.have.been.calledWithMatch('SignalsHandler', 'update', { + clientSignalId: numberSignal.id, + // @ts-expect-error params.event type has id property + event: { id: params1?.event.id, type: 'replace', value: 47, expected: 42 }, + }); + // @ts-expect-error params.event type has id property + simulateReceivingSnapshot(params1?.event.id, 47); + expect(numberSignal.value).to.equal(47); + + numberSignal.increment(-10); + const [, , params2] = client.call.secondCall.args; + expect(client.call).to.have.been.calledWithMatch('SignalsHandler', 'update', { + clientSignalId: numberSignal.id, + // @ts-expect-error params.event type has id property + event: { id: params2?.event.id, type: 'replace', value: 37, expected: 47 }, + }); + // @ts-expect-error params.event type has id property + simulateReceivingSnapshot(params2?.event.id, 37); + expect(numberSignal.value).to.equal(37); + }); + + it('should retry after receiving reject events when add is called and retry on failure is true', () => { + const numberSignal = new NumberSignal(42, config); + subscribeToSignalViaEffect(numberSignal); + + const incrementSubscription = numberSignal.add(-2, true); + expect(client.call).to.have.been.calledOnce; + const [, , params1] = client.call.firstCall.args; + expect(client.call).to.have.been.calledWithMatch('SignalsHandler', 'update', { + clientSignalId: numberSignal.id, + // @ts-expect-error params.event type has id property + event: { id: params1?.event.id, type: 'replace', value: 40, expected: 42 }, + }); + + // @ts-expect-error params.event type has id property + simulateReceivingReject(params1?.event.id); + expect(client.call).to.have.been.calledTwice; + const [, , params2] = client.call.secondCall.args; + expect(client.call).to.have.been.calledWithMatch('SignalsHandler', 'update', { + clientSignalId: numberSignal.id, + // @ts-expect-error params.event type has id property + event: { id: params2?.event.id, type: 'replace', value: 40, expected: 42 }, + }); + + setTimeout(() => incrementSubscription.cancel(), 500); + }); + + it('should not retry after receiving reject events when add is called and retry on failure is false', () => { + const numberSignal = new NumberSignal(42, config); + subscribeToSignalViaEffect(numberSignal); + + numberSignal.add(-2); + expect(client.call).to.have.been.calledOnce; + const [, , params1] = client.call.firstCall.args; + + // @ts-expect-error params.event type has id property + simulateReceivingReject(params1?.event.id); + expect(client.call).to.have.been.calledOnce; + }); + + it('should not send any event to the server when add is called with zero as delta', () => { + const numberSignal = new NumberSignal(42, config); + subscribeToSignalViaEffect(numberSignal); + + numberSignal.add(0); + expect(client.call).not.to.have.been.called; + }); }); }); diff --git a/packages/ts/react-signals/test/ValueSignal.spec.tsx b/packages/ts/react-signals/test/ValueSignal.spec.tsx index 7cab76f0e8..6fb2e2d218 100644 --- a/packages/ts/react-signals/test/ValueSignal.spec.tsx +++ b/packages/ts/react-signals/test/ValueSignal.spec.tsx @@ -8,7 +8,7 @@ import sinonChai from 'sinon-chai'; import type { StateEvent } from '../src/events.js'; import type { ServerConnectionConfig } from '../src/FullStackSignal.js'; import { effect, ValueSignal } from '../src/index.js'; -import { createSubscriptionStub, nextFrame } from './utils.js'; +import { createSubscriptionStub, nextFrame, subscribeToSignalViaEffect } from './utils.js'; use(sinonChai); use(chaiLike); @@ -24,14 +24,6 @@ describe('@vaadin/hilla-react-signals', () => { let subscription: sinon.SinonStubbedInstance>>; let client: sinon.SinonStubbedInstance; - function subscribeToSignalViaEffect(signal: ValueSignal): Array { - const results: Array = []; - effect(() => { - results.push(signal.value); - }); - return results; - } - beforeEach(() => { client = sinon.createStubInstance(ConnectClient); client.call.resolves(); @@ -77,10 +69,8 @@ describe('@vaadin/hilla-react-signals', () => { const valueSignal = new ValueSignal('foo', config); expect(valueSignal.value).to.equal('foo'); - const results: Array = []; - effect(() => { - results.push(valueSignal.value); - }); + const results = subscribeToSignalViaEffect(valueSignal); + valueSignal.value = 'bar'; valueSignal.value += 'baz'; valueSignal.set('qux'); @@ -202,7 +192,7 @@ describe('@vaadin/hilla-react-signals', () => { expect(valueSignal.value).to.equal('c'); // @ts-expect-error params.event type has id property - onNextCallback({ id: params2?.event.id, type: 'reject', value: 'dont care' }); + onNextCallback({ id: params2?.event.id, type: 'reject' }); expect(client.call).to.have.been.calledThrice; const [, , params3] = client.call.thirdCall.args; expect(client.call).to.have.been.calledWithMatch('SignalsHandler', 'update', { diff --git a/packages/ts/react-signals/test/utils.ts b/packages/ts/react-signals/test/utils.ts index 944e9e2245..74a682dfb0 100644 --- a/packages/ts/react-signals/test/utils.ts +++ b/packages/ts/react-signals/test/utils.ts @@ -1,6 +1,7 @@ import type { Subscription } from '@vaadin/hilla-frontend'; import sinon from 'sinon'; import type { StateEvent } from '../src/events.js'; +import { effect, type ValueSignal } from '../src/index.js'; export async function nextFrame(): Promise { return new Promise((resolve) => { @@ -30,3 +31,11 @@ export function createSubscriptionStub(): sinon.SinonStubbedInstance(signal: ValueSignal): Array { + const results: Array = []; + effect(() => { + results.push(signal.value); + }); + return results; +}