Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add increment and add operations to NumberSignal #2694

Merged
merged 32 commits into from
Sep 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
d077c34
refactor(react-signals): base changes
Lodin Aug 22, 2024
98ca34a
refactor(react-signals): use event creators
Lodin Aug 22, 2024
2617950
refactor(react-signals): fix protected methods
Lodin Aug 22, 2024
68df2a2
refactor(react-signals): implement `update` method
Lodin Aug 22, 2024
099d43b
introduce generic ValueSignal type
taefi Aug 22, 2024
d3eee66
Merge branch 'main' into feat/signals/value-signal-impl
taefi Aug 22, 2024
918e248
add tests for ValueSignal
taefi Aug 23, 2024
2a89e50
support having arbitrary types in StateEvent
taefi Aug 23, 2024
b983e21
fix test
taefi Aug 23, 2024
81cd521
Add tests for ValueSignal
taefi Aug 26, 2024
48dface
Add tests for ValueSignal conditional replace
taefi Aug 26, 2024
72f81e4
add tests for ValueSignal.ts
taefi Aug 27, 2024
718b839
Merge branch 'main' into feat/signals/value-signal-impl
taefi Aug 27, 2024
f64730d
add TSDocs for ValueSignal.ts
taefi Aug 27, 2024
fe9f3ef
expose OperationSubscription in index.ts
taefi Aug 27, 2024
de76b47
more detailed tests for update function
taefi Aug 28, 2024
9891627
Merge branch 'main' into feat/signals/value-signal-impl
taefi Aug 28, 2024
669c7da
Allow null as initial and future values
taefi Aug 28, 2024
afee2cc
override get/set for NumberSignal
taefi Aug 28, 2024
28e4927
adapt signal generator plugin to assume null as default value
taefi Aug 28, 2024
d3c0ca3
enable signal generator plugin to generate ValueSignal services
taefi Aug 28, 2024
03fcf72
fix tests
taefi Aug 28, 2024
0433890
rollback changes for accepting null in ValueSignal
taefi Aug 29, 2024
8fe69eb
fix typo in log message
taefi Aug 29, 2024
a6e96ec
Merge branch 'main' into feat/signals/value-signal-impl
taefi Aug 29, 2024
7a3dbdc
Merge branch 'main' into feat/signals/value-signal-impl
taefi Aug 29, 2024
63d6c6e
fix IT according to 0 being the client-side default for NumberSignal
taefi Aug 30, 2024
d7a4c90
Merge branch 'main' into feat/signals/value-signal-impl
taefi Aug 30, 2024
fdc93fa
Merge branch 'main' into feat/signals/value-signal-impl
taefi Sep 2, 2024
22bae09
feat: add increment and add operations to NumberSignal
taefi Sep 2, 2024
58525a8
adjust tests utils subscription to have onDisconnet
taefi Sep 3, 2024
d66fc8a
Merge branch 'main' into taefi/add-atomic-addition-to-numbersignal-2
taefi Sep 3, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
87 changes: 85 additions & 2 deletions packages/ts/react-signals/src/Signals.ts
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -23,4 +23,87 @@ import { ValueSignal } from './ValueSignal.js';
* );
* ```
*/
export class NumberSignal extends ValueSignal<number> {}
export class NumberSignal extends ValueSignal<number> {
/**
* 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;
}
}
195 changes: 174 additions & 21 deletions packages/ts/react-signals/test/Signals.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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<Subscription<StateEvent<number>>>;
let client: sinon.SinonStubbedInstance<ConnectClient>;

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<Subscription<StateEvent<number>>>({
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' };
});
Expand Down Expand Up @@ -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;
});
});
});
18 changes: 4 additions & 14 deletions packages/ts/react-signals/test/ValueSignal.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -24,14 +24,6 @@ describe('@vaadin/hilla-react-signals', () => {
let subscription: sinon.SinonStubbedInstance<Subscription<StateEvent<string>>>;
let client: sinon.SinonStubbedInstance<ConnectClient>;

function subscribeToSignalViaEffect<T>(signal: ValueSignal<T>): Array<T | undefined> {
const results: Array<T | undefined> = [];
effect(() => {
results.push(signal.value);
});
return results;
}

beforeEach(() => {
client = sinon.createStubInstance(ConnectClient);
client.call.resolves();
Expand Down Expand Up @@ -77,10 +69,8 @@ describe('@vaadin/hilla-react-signals', () => {
const valueSignal = new ValueSignal<string>('foo', config);
expect(valueSignal.value).to.equal('foo');

const results: Array<string | null> = [];
effect(() => {
results.push(valueSignal.value);
});
const results = subscribeToSignalViaEffect(valueSignal);

valueSignal.value = 'bar';
valueSignal.value += 'baz';
valueSignal.set('qux');
Expand Down Expand Up @@ -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', {
Expand Down
9 changes: 9 additions & 0 deletions packages/ts/react-signals/test/utils.ts
Original file line number Diff line number Diff line change
@@ -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<void> {
return new Promise<void>((resolve) => {
Expand Down Expand Up @@ -30,3 +31,11 @@ export function createSubscriptionStub<T>(): sinon.SinonStubbedInstance<Subscrip
},
});
}

export function subscribeToSignalViaEffect<T>(signal: ValueSignal<T>): Array<T | undefined> {
const results: Array<T | undefined> = [];
effect(() => {
results.push(signal.value);
});
return results;
}