From 55f84861caea64e0de87404eb522451323083fb7 Mon Sep 17 00:00:00 2001 From: Vlad Rindevich Date: Wed, 7 Aug 2024 16:13:13 +0300 Subject: [PATCH 01/22] refactor(react-signals): simplify approach --- packages/ts/react-signals/src/EventChannel.ts | 85 ++++++++----------- packages/ts/react-signals/src/Signals.ts | 58 +------------ .../ts/react-signals/src/SignalsHandler.ts | 24 ------ packages/ts/react-signals/src/index.ts | 2 +- .../react-signals/test/EventChannel.spec.tsx | 30 ++++--- 5 files changed, 54 insertions(+), 145 deletions(-) delete mode 100644 packages/ts/react-signals/src/SignalsHandler.ts diff --git a/packages/ts/react-signals/src/EventChannel.ts b/packages/ts/react-signals/src/EventChannel.ts index c2bc68234e..cc40d77583 100644 --- a/packages/ts/react-signals/src/EventChannel.ts +++ b/packages/ts/react-signals/src/EventChannel.ts @@ -1,55 +1,47 @@ -import type { ConnectClient, Subscription } from '@vaadin/hilla-frontend'; -import { NumberSignal, setInternalValue, type ValueSignal } from './Signals.js'; -import SignalsHandler from './SignalsHandler'; +import type { ConnectClient } from '@vaadin/hilla-frontend'; +import type { ValueSignal } from './Signals.js'; import { type StateEvent, StateEventType } from './types.js'; -/** - * The type that describes the needed information to - * subscribe and publish to a server-side signal instance. - */ -type SignalChannelDescriptor = Readonly<{ - signalProviderEndpointMethod: string; - subscribe(signalProviderEndpointMethod: string, clientSignalId: string): Subscription; - publish(clientSignalId: string, event: T): Promise; -}>; +const ENDPOINT = 'SignalsHandler'; /** - * A generic class that represents a signal channel - * that can be used to communicate with a server-side - * signal instance. + * A signal channel can be used to communicate with a server-side signal + * instance. * - * The signal channel is responsible for subscribing to - * the server-side signal and updating the local signal - * based on the received events. + * The signal channel is responsible for subscribing to the server-side signal + * and updating the local signal based on the received events. * * @typeParam T - The type of the signal value. * @typeParam S - The type of the signal instance. */ -abstract class SignalChannel> { - readonly #channelDescriptor: SignalChannelDescriptor; - readonly #signalsHandler: SignalsHandler; - readonly #id: string; - +export class SignalChannel> { + readonly #id = crypto.randomUUID(); + readonly #client: ConnectClient; + readonly #signalProviderEndpointMethod: string; readonly #internalSignal: S; + #paused = false; - constructor(signalProviderServiceMethod: string, connectClient: ConnectClient) { - this.#id = crypto.randomUUID(); - this.#signalsHandler = new SignalsHandler(connectClient); - this.#channelDescriptor = { - signalProviderEndpointMethod: signalProviderServiceMethod, - subscribe: (signalProviderEndpointMethod: string, signalId: string) => - this.#signalsHandler.subscribe(signalProviderEndpointMethod, signalId), - publish: async (signalId: string, event: StateEvent) => this.#signalsHandler.update(signalId, event), - }; - - this.#internalSignal = this.createInternalSignal(async (event: StateEvent) => this.publish(event)); + constructor(createSignal: () => S, signalProviderServiceMethod: string, client: ConnectClient) { + this.#client = client; + this.#signalProviderEndpointMethod = signalProviderServiceMethod; + this.#internalSignal = createSignal(); + this.#internalSignal.subscribe((value) => { + if (!this.#paused) { + this.publish({ id: crypto.randomUUID(), type: StateEventType.SET, value }).catch((error) => { + throw error; + }); + } + }); this.#connect(); } #connect() { - this.#channelDescriptor - .subscribe(this.#channelDescriptor.signalProviderEndpointMethod, this.#id) + this.#client + .subscribe(ENDPOINT, 'subscribe', { + signalProviderEndpointMethod: this.#signalProviderEndpointMethod, + clientSignalId: this.#id, + }) .onNext((stateEvent) => { // Update signals based on the new value from the event: this.#updateSignals(stateEvent); @@ -58,12 +50,17 @@ abstract class SignalChannel> { #updateSignals(stateEvent: StateEvent): void { if (stateEvent.type === StateEventType.SNAPSHOT) { - setInternalValue(this.#internalSignal, stateEvent.value); + this.#paused = true; + this.#internalSignal.value = stateEvent.value; + this.#paused = false; } } async publish(event: StateEvent): Promise { - await this.#channelDescriptor.publish(this.#id, event); + await this.#client.call(ENDPOINT, 'update', { + clientSignalId: this.#id, + event, + }); return true; } @@ -80,16 +77,4 @@ abstract class SignalChannel> { get id(): string { return this.#id; } - - abstract createInternalSignal(publish: (event: StateEvent) => Promise, initialValue?: T): S; -} - -/** - * A signal channel that is used to communicate with a - * server-side signal instance that holds a number value. - */ -export class NumberSignalChannel extends SignalChannel { - override createInternalSignal(publish: (event: StateEvent) => Promise, initialValue?: number): NumberSignal { - return new NumberSignal(publish, initialValue); - } } diff --git a/packages/ts/react-signals/src/Signals.ts b/packages/ts/react-signals/src/Signals.ts index 5f478f0ad3..ff9bac2a39 100644 --- a/packages/ts/react-signals/src/Signals.ts +++ b/packages/ts/react-signals/src/Signals.ts @@ -1,8 +1,4 @@ import { Signal } from './core.js'; -import { type StateEvent, StateEventType } from './types'; - -// eslint-disable-next-line import/no-mutable-exports -export let setInternalValue: (signal: ValueSignal, value: T) => void; /** * A signal that holds a value. The underlying @@ -11,59 +7,7 @@ export let setInternalValue: (signal: ValueSignal, value: T) => void; * * @internal */ -export abstract class ValueSignal extends Signal { - static { - setInternalValue = (signal: ValueSignal, value: unknown): void => signal.#setInternalValue(value); - } - - readonly #publish: (event: StateEvent) => Promise; - - /** - * Creates a new ValueSignal instance. - * @param publish - The function that publishes the - * value of the signal to the server. - * @param value - The initial value of the signal - * @defaultValue undefined - */ - constructor(publish: (event: StateEvent) => Promise, value?: T) { - super(value); - this.#publish = publish; - } - - /** - * Returns the value of the signal. - */ - override get value(): T { - return super.value; - } - - /** - * Publishes the new value to the server. - * Note that this method is not setting - * the signal's value. - * - * @param value - The new value of the signal - * to be published to the server. - */ - override set value(value: T) { - const id = crypto.randomUUID(); - // set the local value to be used for latency compensation and offline support: - this.#setInternalValue(value); - // publish the update to the server: - this.#publish({ id, type: StateEventType.SET, value }).catch((error) => { - throw error; - }); - } - - /** - * Sets the value of the signal. - * @param value - The new value of the signal. - * @internal - */ - #setInternalValue(value: T): void { - super.value = value; - } -} +export abstract class ValueSignal extends Signal {} /** * A signal that holds a number value. The underlying diff --git a/packages/ts/react-signals/src/SignalsHandler.ts b/packages/ts/react-signals/src/SignalsHandler.ts deleted file mode 100644 index d54929e5e4..0000000000 --- a/packages/ts/react-signals/src/SignalsHandler.ts +++ /dev/null @@ -1,24 +0,0 @@ -import type { ConnectClient, EndpointRequestInit, Subscription } from '@vaadin/hilla-frontend'; -import type { StateEvent } from './types'; - -/** - * SignalsHandler is a helper class for handling the - * communication of the full-stack signal instances - * and their server-side counterparts they are - * subscribed and publish their updates to. - */ -export default class SignalsHandler { - readonly #client: ConnectClient; - - constructor(client: ConnectClient) { - this.#client = client; - } - - subscribe(signalProviderEndpointMethod: string, clientSignalId: string): Subscription { - return this.#client.subscribe('SignalsHandler', 'subscribe', { signalProviderEndpointMethod, clientSignalId }); - } - - async update(clientSignalId: string, event: StateEvent, init?: EndpointRequestInit): Promise { - return this.#client.call('SignalsHandler', 'update', { clientSignalId, event }, init); - } -} diff --git a/packages/ts/react-signals/src/index.ts b/packages/ts/react-signals/src/index.ts index a0e10b38ce..c1963c8651 100644 --- a/packages/ts/react-signals/src/index.ts +++ b/packages/ts/react-signals/src/index.ts @@ -1,5 +1,5 @@ // eslint-disable-next-line import/export export * from './core.js'; -export { NumberSignalChannel } from './EventChannel.js'; +export { SignalChannel } from './EventChannel.js'; export { NumberSignal, ValueSignal } from './Signals.js'; export * from './types.js'; diff --git a/packages/ts/react-signals/test/EventChannel.spec.tsx b/packages/ts/react-signals/test/EventChannel.spec.tsx index 83a72a685f..da5e553706 100644 --- a/packages/ts/react-signals/test/EventChannel.spec.tsx +++ b/packages/ts/react-signals/test/EventChannel.spec.tsx @@ -4,19 +4,23 @@ import { render } from '@testing-library/react'; import { ConnectClient, type Subscription } from '@vaadin/hilla-frontend'; import sinon from 'sinon'; import sinonChai from 'sinon-chai'; -import { NumberSignal, NumberSignalChannel, type StateEvent, StateEventType } from '../src/index.js'; +import { NumberSignal, SignalChannel, type StateEvent, StateEventType } from '../src/index.js'; import { nextFrame } from './utils.js'; use(sinonChai); -function simulateReceivedEvent(connectSubscriptionMock: Subscription, event: StateEvent) { - const onNextCallback = (connectSubscriptionMock.onNext as sinon.SinonStub).getCall(0).args[0]; - // eslint-disable-next-line @typescript-eslint/no-unsafe-call - onNextCallback(event); -} - describe('@vaadin/hilla-react-signals', () => { - describe('NumberSignalChannel', () => { + describe('SignalChannel', () => { + function simulateReceivedEvent(connectSubscriptionMock: Subscription, event: StateEvent) { + const onNextCallback = (connectSubscriptionMock.onNext as sinon.SinonStub).getCall(0).args[0]; + // eslint-disable-next-line @typescript-eslint/no-unsafe-call + onNextCallback(event); + } + + function createNumberSignal() { + return new NumberSignal(); + } + let connectClientMock: sinon.SinonStubbedInstance; let connectSubscriptionMock: Subscription; @@ -39,13 +43,13 @@ describe('@vaadin/hilla-react-signals', () => { }); it('should create signal instance of type NumberSignal', () => { - const numberSignalChannel = new NumberSignalChannel('testEndpoint', connectClientMock); + const numberSignalChannel = new SignalChannel(createNumberSignal, 'testEndpoint', connectClientMock); expect(numberSignalChannel.signal).to.be.instanceOf(NumberSignal); expect(numberSignalChannel.signal.value).to.be.undefined; }); it('should subscribe to signal provider endpoint', () => { - const numberSignalChannel = new NumberSignalChannel('testEndpoint', connectClientMock); + const numberSignalChannel = new SignalChannel(createNumberSignal, 'testEndpoint', connectClientMock); expect(connectClientMock.subscribe).to.be.have.been.calledOnce; expect(connectClientMock.subscribe).to.have.been.calledWith('SignalsHandler', 'subscribe', { clientSignalId: numberSignalChannel.id, @@ -54,7 +58,7 @@ describe('@vaadin/hilla-react-signals', () => { }); it('should publish updates to signals handler endpoint', () => { - const numberSignalChannel = new NumberSignalChannel('testEndpoint', connectClientMock); + const numberSignalChannel = new SignalChannel(createNumberSignal, 'testEndpoint', connectClientMock); numberSignalChannel.signal.value = 42; expect(connectClientMock.call).to.be.have.been.calledOnce; @@ -70,7 +74,7 @@ describe('@vaadin/hilla-react-signals', () => { }); it("should update signal's value based on the received event", () => { - const numberSignalChannel = new NumberSignalChannel('testEndpoint', connectClientMock); + const numberSignalChannel = new SignalChannel(createNumberSignal, 'testEndpoint', connectClientMock); expect(numberSignalChannel.signal.value).to.be.undefined; // Simulate the event received from the server: @@ -82,7 +86,7 @@ describe('@vaadin/hilla-react-signals', () => { }); it("should render signal's the updated value", async () => { - const numberSignalChannel = new NumberSignalChannel('testEndpoint', connectClientMock); + const numberSignalChannel = new SignalChannel(createNumberSignal, 'testEndpoint', connectClientMock); const numberSignal = numberSignalChannel.signal; simulateReceivedEvent(connectSubscriptionMock, { id: 'someId', type: StateEventType.SNAPSHOT, value: 42 }); From 1755cd959221470714d668bc68fc68271e7f5e3c Mon Sep 17 00:00:00 2001 From: Vlad Rindevich Date: Thu, 8 Aug 2024 21:22:08 +0300 Subject: [PATCH 02/22] refactor(react-signals): simplify code --- package-lock.json | 21 ++- packages/ts/react-signals/package.json | 3 +- packages/ts/react-signals/src/EventChannel.ts | 80 ---------- .../ts/react-signals/src/SignalChannel.ts | 79 ++++++++++ packages/ts/react-signals/src/index.ts | 3 +- packages/ts/react-signals/src/types.ts | 16 -- .../react-signals/test/EventChannel.spec.tsx | 102 ------------- .../react-signals/test/SignalChannel.spec.tsx | 138 ++++++++++++++++++ .../ts/react-signals/test/Signals.spec.tsx | 43 +----- .../react-signals/test/SignalsHandler.spec.ts | 53 ------- 10 files changed, 248 insertions(+), 290 deletions(-) delete mode 100644 packages/ts/react-signals/src/EventChannel.ts create mode 100644 packages/ts/react-signals/src/SignalChannel.ts delete mode 100644 packages/ts/react-signals/src/types.ts delete mode 100644 packages/ts/react-signals/test/EventChannel.spec.tsx create mode 100644 packages/ts/react-signals/test/SignalChannel.spec.tsx delete mode 100644 packages/ts/react-signals/test/SignalsHandler.spec.ts diff --git a/package-lock.json b/package-lock.json index 36ccc8bf9f..606550da5b 100644 --- a/package-lock.json +++ b/package-lock.json @@ -18799,7 +18799,8 @@ "license": "Apache-2.0", "dependencies": { "@preact/signals-react": "^2.0.0", - "@vaadin/hilla-frontend": "24.5.0-alpha7" + "@vaadin/hilla-frontend": "24.5.0-alpha7", + "nanoid": "^5.0.7" }, "devDependencies": { "@esm-bundle/chai": "^4.3.4-fix.0", @@ -18854,6 +18855,24 @@ "optional": true } } + }, + "packages/ts/react-signals/node_modules/nanoid": { + "version": "5.0.7", + "resolved": "https://registry.npmjs.org/nanoid/-/nanoid-5.0.7.tgz", + "integrity": "sha512-oLxFY2gd2IqnjcYyOXD8XGCftpGtZP2AbHbOkthDkvRywH5ayNtPVy9YlOPcHckXzbLTCHpkb7FB+yuxKV13pQ==", + "funding": [ + { + "type": "github", + "url": "https://github.com/sponsors/ai" + } + ], + "license": "MIT", + "bin": { + "nanoid": "bin/nanoid.js" + }, + "engines": { + "node": "^18 || >=20" + } } } } diff --git a/packages/ts/react-signals/package.json b/packages/ts/react-signals/package.json index a4827612ce..e79ee07e6f 100644 --- a/packages/ts/react-signals/package.json +++ b/packages/ts/react-signals/package.json @@ -47,7 +47,8 @@ }, "dependencies": { "@preact/signals-react": "^2.0.0", - "@vaadin/hilla-frontend": "24.5.0-alpha7" + "@vaadin/hilla-frontend": "24.5.0-alpha7", + "nanoid": "^5.0.7" }, "peerDependencies": { "react": "^18", diff --git a/packages/ts/react-signals/src/EventChannel.ts b/packages/ts/react-signals/src/EventChannel.ts deleted file mode 100644 index cc40d77583..0000000000 --- a/packages/ts/react-signals/src/EventChannel.ts +++ /dev/null @@ -1,80 +0,0 @@ -import type { ConnectClient } from '@vaadin/hilla-frontend'; -import type { ValueSignal } from './Signals.js'; -import { type StateEvent, StateEventType } from './types.js'; - -const ENDPOINT = 'SignalsHandler'; - -/** - * A signal channel can be used to communicate with a server-side signal - * instance. - * - * The signal channel is responsible for subscribing to the server-side signal - * and updating the local signal based on the received events. - * - * @typeParam T - The type of the signal value. - * @typeParam S - The type of the signal instance. - */ -export class SignalChannel> { - readonly #id = crypto.randomUUID(); - readonly #client: ConnectClient; - readonly #signalProviderEndpointMethod: string; - readonly #internalSignal: S; - #paused = false; - - constructor(createSignal: () => S, signalProviderServiceMethod: string, client: ConnectClient) { - this.#client = client; - this.#signalProviderEndpointMethod = signalProviderServiceMethod; - this.#internalSignal = createSignal(); - this.#internalSignal.subscribe((value) => { - if (!this.#paused) { - this.publish({ id: crypto.randomUUID(), type: StateEventType.SET, value }).catch((error) => { - throw error; - }); - } - }); - - this.#connect(); - } - - #connect() { - this.#client - .subscribe(ENDPOINT, 'subscribe', { - signalProviderEndpointMethod: this.#signalProviderEndpointMethod, - clientSignalId: this.#id, - }) - .onNext((stateEvent) => { - // Update signals based on the new value from the event: - this.#updateSignals(stateEvent); - }); - } - - #updateSignals(stateEvent: StateEvent): void { - if (stateEvent.type === StateEventType.SNAPSHOT) { - this.#paused = true; - this.#internalSignal.value = stateEvent.value; - this.#paused = false; - } - } - - async publish(event: StateEvent): Promise { - await this.#client.call(ENDPOINT, 'update', { - clientSignalId: this.#id, - event, - }); - return true; - } - - /** - * Returns the signal instance to be used in components. - */ - get signal(): S { - return this.#internalSignal; - } - - /** - * Returns the id of the signal channel. - */ - get id(): string { - return this.#id; - } -} diff --git a/packages/ts/react-signals/src/SignalChannel.ts b/packages/ts/react-signals/src/SignalChannel.ts new file mode 100644 index 0000000000..b382ba0fa9 --- /dev/null +++ b/packages/ts/react-signals/src/SignalChannel.ts @@ -0,0 +1,79 @@ +import type { ConnectClient, Subscription } from '@vaadin/hilla-frontend'; +import { nanoid } from 'nanoid'; +import type { ValueSignal } from './Signals.js'; + +const ENDPOINT = 'SignalsHandler'; + +/** + * Types of changes that can be produced or processed by a signal. + */ +export enum StateChangeType { + SET = 'set', + SNAPSHOT = 'snapshot', +} + +/** + * An object that describes the change of the signal state. + */ +export type StateChange = Readonly<{ + id: string; + type: StateChangeType; + value: T; +}>; + +export type Value> = S extends ValueSignal ? T : never; + +export type SignalChannel> = Readonly<{ + id: string; + subscription: Subscription>>; +}>; + +/** + * Creates a signal channel that can be used to communicate with a server-side. + * + * The signal channel is responsible for subscribing to the server-side signal + * and updating the local signal based on the received events. + * + * @typeParam S - The type of the signal instance. + * @param signal - The local signal instance to be updated. + * @param signalProviderEndpointMethod - The method name of the signal provider + * service. + * @param client - The client instance to be used for communication. + * @returns The id of the created signal channel. + */ +export function createSignalChannel>( + signal: S, + signalProviderEndpointMethod: string, + client: ConnectClient, +): SignalChannel { + const id = nanoid(); + + let paused = false; + signal.subscribe((value) => { + if (!paused && value) { + client + .call(ENDPOINT, 'update', { + clientSignalId: id, + change: { id: nanoid(), type: StateChangeType.SET, value }, + }) + .catch((error) => { + throw error; + }); + } + }); + + const subscription: Subscription>> = client + .subscribe(ENDPOINT, 'subscribe', { + signalProviderEndpointMethod, + clientSignalId: id, + }) + .onNext((change: StateChange>) => { + if (change.type === StateChangeType.SNAPSHOT) { + paused = true; + signal.value = change.value; + paused = false; + } + }); + + return { id, subscription }; +} diff --git a/packages/ts/react-signals/src/index.ts b/packages/ts/react-signals/src/index.ts index c1963c8651..3da9f3af0a 100644 --- a/packages/ts/react-signals/src/index.ts +++ b/packages/ts/react-signals/src/index.ts @@ -1,5 +1,4 @@ // eslint-disable-next-line import/export export * from './core.js'; -export { SignalChannel } from './EventChannel.js'; +export { createSignalChannel, type SignalChannel } from './SignalChannel.js'; export { NumberSignal, ValueSignal } from './Signals.js'; -export * from './types.js'; diff --git a/packages/ts/react-signals/src/types.ts b/packages/ts/react-signals/src/types.ts deleted file mode 100644 index 19e4a884ab..0000000000 --- a/packages/ts/react-signals/src/types.ts +++ /dev/null @@ -1,16 +0,0 @@ -/** - * Types of events that can be produced or processed by a signal. - */ -export enum StateEventType { - SET = 'set', - SNAPSHOT = 'snapshot', -} - -/** - * Event that describes the state of a signal. - */ -export type StateEvent = { - id: string; - type: StateEventType; - value: any; -}; diff --git a/packages/ts/react-signals/test/EventChannel.spec.tsx b/packages/ts/react-signals/test/EventChannel.spec.tsx deleted file mode 100644 index da5e553706..0000000000 --- a/packages/ts/react-signals/test/EventChannel.spec.tsx +++ /dev/null @@ -1,102 +0,0 @@ -/* eslint-disable @typescript-eslint/unbound-method */ -import { expect, use } from '@esm-bundle/chai'; -import { render } from '@testing-library/react'; -import { ConnectClient, type Subscription } from '@vaadin/hilla-frontend'; -import sinon from 'sinon'; -import sinonChai from 'sinon-chai'; -import { NumberSignal, SignalChannel, type StateEvent, StateEventType } from '../src/index.js'; -import { nextFrame } from './utils.js'; - -use(sinonChai); - -describe('@vaadin/hilla-react-signals', () => { - describe('SignalChannel', () => { - function simulateReceivedEvent(connectSubscriptionMock: Subscription, event: StateEvent) { - const onNextCallback = (connectSubscriptionMock.onNext as sinon.SinonStub).getCall(0).args[0]; - // eslint-disable-next-line @typescript-eslint/no-unsafe-call - onNextCallback(event); - } - - function createNumberSignal() { - return new NumberSignal(); - } - - let connectClientMock: sinon.SinonStubbedInstance; - let connectSubscriptionMock: Subscription; - - beforeEach(() => { - connectClientMock = sinon.createStubInstance(ConnectClient); - connectClientMock.call.resolves(); - connectSubscriptionMock = { - cancel: sinon.stub(), - context: sinon.stub().returnsThis(), - onComplete: sinon.stub().returnsThis(), - onError: sinon.stub().returnsThis(), - onNext: sinon.stub().returnsThis(), - }; - // Mock the subscribe method - connectClientMock.subscribe.returns(connectSubscriptionMock); - }); - - afterEach(() => { - sinon.restore(); - }); - - it('should create signal instance of type NumberSignal', () => { - const numberSignalChannel = new SignalChannel(createNumberSignal, 'testEndpoint', connectClientMock); - expect(numberSignalChannel.signal).to.be.instanceOf(NumberSignal); - expect(numberSignalChannel.signal.value).to.be.undefined; - }); - - it('should subscribe to signal provider endpoint', () => { - const numberSignalChannel = new SignalChannel(createNumberSignal, 'testEndpoint', connectClientMock); - expect(connectClientMock.subscribe).to.be.have.been.calledOnce; - expect(connectClientMock.subscribe).to.have.been.calledWith('SignalsHandler', 'subscribe', { - clientSignalId: numberSignalChannel.id, - signalProviderEndpointMethod: 'testEndpoint', - }); - }); - - it('should publish updates to signals handler endpoint', () => { - const numberSignalChannel = new SignalChannel(createNumberSignal, 'testEndpoint', connectClientMock); - numberSignalChannel.signal.value = 42; - - expect(connectClientMock.call).to.be.have.been.calledOnce; - expect(connectClientMock.call).to.have.been.calledWithMatch( - 'SignalsHandler', - 'update', - { - clientSignalId: numberSignalChannel.id, - event: { type: StateEventType.SET, value: 42 }, - }, - undefined, - ); - }); - - it("should update signal's value based on the received event", () => { - const numberSignalChannel = new SignalChannel(createNumberSignal, 'testEndpoint', connectClientMock); - expect(numberSignalChannel.signal.value).to.be.undefined; - - // Simulate the event received from the server: - const snapshotEvent: StateEvent = { id: 'someId', type: StateEventType.SNAPSHOT, value: 42 }; - simulateReceivedEvent(connectSubscriptionMock, snapshotEvent); - - // Check if the signal value is updated: - expect(numberSignalChannel.signal.value).to.equal(42); - }); - - it("should render signal's the updated value", async () => { - const numberSignalChannel = new SignalChannel(createNumberSignal, 'testEndpoint', connectClientMock); - const numberSignal = numberSignalChannel.signal; - simulateReceivedEvent(connectSubscriptionMock, { id: 'someId', type: StateEventType.SNAPSHOT, value: 42 }); - - const result = render(Value is {numberSignal}); - await nextFrame(); - expect(result.container.textContent).to.equal('Value is 42'); - - simulateReceivedEvent(connectSubscriptionMock, { id: 'someId', type: StateEventType.SNAPSHOT, value: 99 }); - await nextFrame(); - expect(result.container.textContent).to.equal('Value is 99'); - }); - }); -}); diff --git a/packages/ts/react-signals/test/SignalChannel.spec.tsx b/packages/ts/react-signals/test/SignalChannel.spec.tsx new file mode 100644 index 0000000000..847b19020c --- /dev/null +++ b/packages/ts/react-signals/test/SignalChannel.spec.tsx @@ -0,0 +1,138 @@ +/* eslint-disable @typescript-eslint/unbound-method */ +import { expect, use } from '@esm-bundle/chai'; +import { render } from '@testing-library/react'; +import { ConnectClient, type Subscription } from '@vaadin/hilla-frontend'; +import sinon from 'sinon'; +import sinonChai from 'sinon-chai'; +import { NumberSignal, type SignalChannel } from '../src/index.js'; +import { createSignalChannel, type StateChange, StateChangeType } from '../src/SignalChannel.js'; +import { nextFrame } from './utils.js'; + +use(sinonChai); + +describe('@vaadin/hilla-react-signals', () => { + describe('SignalChannel', () => { + function simulateReceivedChange( + connectSubscriptionMock: sinon.SinonSpiedInstance>>, + change: StateChange, + ) { + const onNextCallback = connectSubscriptionMock.onNext.getCall(0).args[0]; + // eslint-disable-next-line @typescript-eslint/no-unsafe-call + onNextCallback(change); + } + + let connectClientMock: sinon.SinonStubbedInstance; + let connectSubscriptionMock: sinon.SinonSpiedInstance>>; + let signal: NumberSignal; + let channel: SignalChannel; + + beforeEach(() => { + connectClientMock = sinon.createStubInstance(ConnectClient); + connectClientMock.call.resolves(); + + connectSubscriptionMock = sinon.spy>>({ + cancel() {}, + context() { + return this; + }, + onComplete() { + return this; + }, + onError() { + return this; + }, + onNext() { + return this; + }, + }); + // Mock the subscribe method + connectClientMock.subscribe.returns(connectSubscriptionMock); + + signal = new NumberSignal(); + channel = createSignalChannel(signal, 'testEndpoint', connectClientMock); + connectClientMock.call.resetHistory(); + }); + + afterEach(() => { + sinon.resetHistory(); + }); + + it('should create signal instance of type NumberSignal', () => { + expect(signal).to.be.instanceOf(NumberSignal); + expect(signal.value).to.be.undefined; + }); + + it('should subscribe to signal provider endpoint', () => { + expect(connectClientMock.subscribe).to.be.have.been.calledOnce; + expect(connectClientMock.subscribe).to.have.been.calledWith('SignalsHandler', 'subscribe', { + clientSignalId: channel.id, + signalProviderEndpointMethod: 'testEndpoint', + }); + }); + + it('should publish updates to signals handler endpoint', () => { + signal.value = 42; + + expect(connectClientMock.call).to.be.have.been.calledOnce; + expect(connectClientMock.call).to.have.been.calledWithMatch('SignalsHandler', 'update', { + clientSignalId: channel.id, + change: { type: StateChangeType.SET, value: 42 }, + }); + }); + + it("should update signal's value based on the received event", () => { + expect(signal.value).to.be.undefined; + + // Simulate the event received from the server: + const snapshotEvent: StateChange = { id: 'someId', type: StateChangeType.SNAPSHOT, value: 42 }; + simulateReceivedChange(connectSubscriptionMock, snapshotEvent); + + // Check if the signal value is updated: + expect(signal.value).to.equal(42); + }); + + it('should render the updated value', async () => { + const numberSignal = signal; + simulateReceivedChange(connectSubscriptionMock, { id: 'someId', type: StateChangeType.SNAPSHOT, value: 42 }); + + const result = render(Value is {numberSignal}); + await nextFrame(); + expect(result.container.textContent).to.equal('Value is 42'); + + simulateReceivedChange(connectSubscriptionMock, { id: 'someId', type: StateChangeType.SNAPSHOT, value: 99 }); + await nextFrame(); + expect(result.container.textContent).to.equal('Value is 99'); + }); + + it('should subscribe using client', () => { + expect(connectClientMock.subscribe).to.be.have.been.calledOnce; + expect(connectClientMock.subscribe).to.have.been.calledWith('SignalsHandler', 'subscribe', { + signalProviderEndpointMethod: 'testEndpoint', + clientSignalId: channel.id, + }); + }); + + it('should publish the new value to the server when set', () => { + signal.value = 42; + expect(connectClientMock.call).to.have.been.calledOnce; + expect(connectClientMock.call).to.have.been.calledWithMatch('SignalsHandler', 'update', { + change: { type: StateChangeType.SET, value: 42 }, + }); + + signal.value = 0; + + connectClientMock.call.resetHistory(); + + signal.value += 1; + expect(connectClientMock.call).to.have.been.calledOnce; + expect(connectClientMock.call).to.have.been.calledWithMatch('SignalsHandler', 'update', { + change: { type: StateChangeType.SET, value: 1 }, + }); + }); + + it('should throw an error when the server call fails', async () => { + connectClientMock.call.rejects(new Error('Server error')); + signal.value = 42; + }); + }); +}); diff --git a/packages/ts/react-signals/test/Signals.spec.tsx b/packages/ts/react-signals/test/Signals.spec.tsx index 26dc7bde02..55e1900405 100644 --- a/packages/ts/react-signals/test/Signals.spec.tsx +++ b/packages/ts/react-signals/test/Signals.spec.tsx @@ -2,9 +2,7 @@ import { expect, use } from '@esm-bundle/chai'; import { render } from '@testing-library/react'; import chaiLike from 'chai-like'; -import sinon from 'sinon'; import sinonChai from 'sinon-chai'; -import type { StateEvent } from '../src'; import { effect } from '../src'; import { NumberSignal } from '../src'; import { nextFrame } from './utils.js'; @@ -14,62 +12,37 @@ use(chaiLike); describe('@vaadin/hilla-react-signals', () => { describe('NumberSignal', () => { - let publishSpy: sinon.SinonSpy; - - beforeEach(() => { - publishSpy = sinon.spy(async (_: StateEvent): Promise => Promise.resolve(true)); - }); - - afterEach(() => { - sinon.restore(); - }); - it('should retain default value as initialized', () => { - const numberSignal1 = new NumberSignal(publishSpy); + const numberSignal1 = new NumberSignal(); expect(numberSignal1.value).to.be.undefined; - const numberSignal2 = new NumberSignal(publishSpy, undefined); + const numberSignal2 = new NumberSignal(undefined); expect(numberSignal2.value).to.be.undefined; - const numberSignal3 = new NumberSignal(publishSpy, 0); + const numberSignal3 = new NumberSignal(0); expect(numberSignal3.value).to.equal(0); - const numberSignal4 = new NumberSignal(publishSpy, 42.424242); + const numberSignal4 = new NumberSignal(42.424242); expect(numberSignal4.value).to.equal(42.424242); - const numberSignal5 = new NumberSignal(publishSpy, -42.424242); + const numberSignal5 = new NumberSignal(-42.424242); expect(numberSignal5.value).to.equal(-42.424242); }); - it('should publish the new value to the server when set', () => { - const numberSignal = new NumberSignal(publishSpy); - numberSignal.value = 42; - expect(publishSpy).to.have.been.calledOnce; - expect(publishSpy).to.have.been.calledWithMatch({ type: 'set', value: 42 }); - - publishSpy.resetHistory(); - - const numberSignal2 = new NumberSignal(publishSpy, 0); - // eslint-disable-next-line no-plusplus - numberSignal2.value++; - expect(publishSpy).to.have.been.calledOnce; - expect(publishSpy).to.have.been.calledWithMatch({ type: 'set', value: 1 }); - }); - it('should render value when signal is rendered', async () => { - const numberSignal = new NumberSignal(publishSpy, 42); + const numberSignal = new NumberSignal(42); const result = render(Value is {numberSignal}); await nextFrame(); expect(result.container.textContent).to.equal('Value is 42'); }); it('should set the underlying value locally without waiting for server confirmation', () => { - const numberSignal = new NumberSignal(publishSpy); + const numberSignal = new NumberSignal(); expect(numberSignal.value).to.equal(undefined); numberSignal.value = 42; expect(numberSignal.value).to.equal(42); - const anotherNumberSignal = new NumberSignal(publishSpy); + const anotherNumberSignal = new NumberSignal(); const results: Array = []; effect(() => { diff --git a/packages/ts/react-signals/test/SignalsHandler.spec.ts b/packages/ts/react-signals/test/SignalsHandler.spec.ts deleted file mode 100644 index c8dcff3326..0000000000 --- a/packages/ts/react-signals/test/SignalsHandler.spec.ts +++ /dev/null @@ -1,53 +0,0 @@ -/* eslint-disable @typescript-eslint/unbound-method */ -import { expect, use } from '@esm-bundle/chai'; -import { ConnectClient } from '@vaadin/hilla-frontend'; -import sinon from 'sinon'; -import sinonChai from 'sinon-chai'; -import { type StateEvent, StateEventType } from '../src/index.js'; -import SignalsHandler from '../src/SignalsHandler.js'; - -use(sinonChai); - -describe('@vaadin/hilla-react-signals', () => { - describe('signalsHandler', () => { - let connectClientMock: sinon.SinonStubbedInstance; - let signalsHandler: SignalsHandler; - - beforeEach(() => { - connectClientMock = sinon.createStubInstance(ConnectClient); - signalsHandler = new SignalsHandler(connectClientMock); - }); - - afterEach(() => { - sinon.restore(); - }); - - it('subscribe should call client.subscribe', () => { - const signalProviderEndpointMethod = 'testEndpoint'; - const clientSignalId = 'testSignalId'; - signalsHandler.subscribe(signalProviderEndpointMethod, clientSignalId); - - expect(connectClientMock.subscribe).to.be.have.been.calledOnce; - expect(connectClientMock.subscribe).to.have.been.calledWith('SignalsHandler', 'subscribe', { - signalProviderEndpointMethod, - clientSignalId, - }); - }); - - it('update should call client.call', async () => { - const clientSignalId = 'testSignalId'; - const event: StateEvent = { id: 'testEvent', type: StateEventType.SET, value: 10 }; - const init = {}; - - await signalsHandler.update(clientSignalId, event, init); - - expect(connectClientMock.call).to.be.have.been.calledOnce; - expect(connectClientMock.call).to.have.been.calledWith( - 'SignalsHandler', - 'update', - { clientSignalId, event }, - init, - ); - }); - }); -}); From 05be3a1759ca7128818a686565be3bdc8da5ed6c Mon Sep 17 00:00:00 2001 From: Vlad Rindevich Date: Mon, 12 Aug 2024 18:09:14 +0300 Subject: [PATCH 03/22] feat(generator-utils): add traverse function for TS AST --- packages/ts/generator-utils/src/ast.ts | 8 ++++++++ packages/ts/generator-utils/src/createSourceFile.ts | 2 +- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/packages/ts/generator-utils/src/ast.ts b/packages/ts/generator-utils/src/ast.ts index 09fd6ea265..8daf0e9126 100644 --- a/packages/ts/generator-utils/src/ast.ts +++ b/packages/ts/generator-utils/src/ast.ts @@ -60,3 +60,11 @@ export function transform( return ts.visitEachChild(root, visitor, context); }; } + +export function traverse(node: Node, visitor: (node: Node) => T | undefined): T | undefined { + function _visitor(n: Node): T | undefined { + return visitor(n) ?? ts.forEachChild(n, _visitor); + } + + return _visitor(node); +} diff --git a/packages/ts/generator-utils/src/createSourceFile.ts b/packages/ts/generator-utils/src/createSourceFile.ts index 3f5f22e4b3..145599ad37 100644 --- a/packages/ts/generator-utils/src/createSourceFile.ts +++ b/packages/ts/generator-utils/src/createSourceFile.ts @@ -1,6 +1,6 @@ import ts, { type SourceFile, type Statement } from 'typescript'; export default function createSourceFile(statements: readonly Statement[], fileName: string): SourceFile { - const sourceFile = ts.createSourceFile(fileName, '', ts.ScriptTarget.ES2019, undefined, ts.ScriptKind.TS); + const sourceFile = ts.createSourceFile(fileName, '', ts.ScriptTarget.ES2021, undefined, ts.ScriptKind.TS); return ts.factory.updateSourceFile(sourceFile, statements); } From f3171ca7fb8aba344aa1cadca931a4c55cf9f0f0 Mon Sep 17 00:00:00 2001 From: Vlad Rindevich Date: Mon, 12 Aug 2024 18:09:54 +0300 Subject: [PATCH 04/22] refactor(generator-utils): update generator to generate functional `createSignalChannel` --- .../src/SignalProcessor.ts | 67 ++++++++++--------- .../fixtures/NumberSignalServiceMix.snap.ts | 14 +++- .../NumberSignalServiceSignalOnly.snap.ts | 14 +++- 3 files changed, 59 insertions(+), 36 deletions(-) diff --git a/packages/ts/generator-plugin-signals/src/SignalProcessor.ts b/packages/ts/generator-plugin-signals/src/SignalProcessor.ts index 640b6d6ac0..ab92cad66b 100644 --- a/packages/ts/generator-plugin-signals/src/SignalProcessor.ts +++ b/packages/ts/generator-plugin-signals/src/SignalProcessor.ts @@ -1,16 +1,17 @@ import type Plugin from '@vaadin/hilla-generator-core/Plugin.js'; -import { template, transform } from '@vaadin/hilla-generator-utils/ast.js'; +import { template, transform, traverse } from '@vaadin/hilla-generator-utils/ast.js'; import createSourceFile from '@vaadin/hilla-generator-utils/createSourceFile.js'; import DependencyManager from '@vaadin/hilla-generator-utils/dependencies/DependencyManager.js'; import PathManager from '@vaadin/hilla-generator-utils/dependencies/PathManager.js'; -import ts, { type FunctionDeclaration, type SourceFile } from 'typescript'; +import ts, { type FunctionDeclaration, type Identifier, type SourceFile, type TypeNode } from 'typescript'; const HILLA_REACT_SIGNALS = '@vaadin/hilla-react-signals'; -const NUMBER_SIGNAL_CHANNEL = '$NUMBER_SIGNAL_CHANNEL$'; +const CREATE_SIGNAL_CHANNEL = '$CREATE_SIGNAL_CHANNEL$'; const CONNECT_CLIENT = '$CONNECT_CLIENT$'; +const SIGNAL = '$SIGNAL$'; -const signalImportPaths = ['com/vaadin/hilla/signals/NumberSignal']; +const signals = ['NumberSignal', 'ValueSignal']; export default class SignalProcessor { readonly #dependencyManager: DependencyManager; @@ -31,11 +32,11 @@ export default class SignalProcessor { process(): SourceFile { this.#owner.logger.debug(`Processing signals: ${this.#service}`); const { imports } = this.#dependencyManager; - const numberSignalChannelId = imports.named.add(HILLA_REACT_SIGNALS, 'NumberSignalChannel'); + + const createSignalChannelId = imports.named.add(HILLA_REACT_SIGNALS, 'createSignalChannel'); const [, connectClientId] = imports.default.iter().find(([path]) => path.includes('connect-client'))!; - this.#processSignalImports(signalImportPaths); const initTypeId = imports.named.getIdentifier('@vaadin/hilla-frontend', 'EndpointRequestInit'); let initTypeUsageCount = 0; @@ -43,41 +44,33 @@ export default class SignalProcessor { transform((tsNode) => { if (ts.isFunctionDeclaration(tsNode) && tsNode.name && this.#methods.has(tsNode.name.text)) { const methodName = tsNode.name.text; + const signalId = this.#replaceSignalImport(tsNode); const body = template( ` function dummy() { - return new ${NUMBER_SIGNAL_CHANNEL}('${this.#service}.${methodName}', ${CONNECT_CLIENT}).signal; + const signal = new ${SIGNAL}(); + ${CREATE_SIGNAL_CHANNEL}(signal, '${this.#service}.${methodName}', ${CONNECT_CLIENT}); + return signal; }`, (statements) => (statements[0] as FunctionDeclaration).body?.statements, [ transform((node) => - ts.isIdentifier(node) && node.text === NUMBER_SIGNAL_CHANNEL ? numberSignalChannelId : node, + ts.isIdentifier(node) && node.text === CREATE_SIGNAL_CHANNEL ? createSignalChannelId : node, ), + transform((node) => (ts.isIdentifier(node) && node.text === SIGNAL ? signalId : node)), transform((node) => (ts.isIdentifier(node) && node.text === CONNECT_CLIENT ? connectClientId : node)), ], ); - let returnType = tsNode.type; - if ( - returnType && - ts.isTypeReferenceNode(returnType) && - 'text' in returnType.typeName && - returnType.typeName.text === 'Promise' - ) { - if (returnType.typeArguments && returnType.typeArguments.length > 0) { - returnType = returnType.typeArguments[0]; - } - } - return ts.factory.createFunctionDeclaration( tsNode.modifiers?.filter((modifier) => modifier.kind !== ts.SyntaxKind.AsyncKeyword), tsNode.asteriskToken, tsNode.name, tsNode.typeParameters, tsNode.parameters.filter(({ name }) => !(ts.isIdentifier(name) && name.text === 'init')), - returnType, - ts.factory.createBlock(body ?? [], false), + ts.factory.createTypeReferenceNode(signalId), + ts.factory.createBlock(body ?? [], true), ); } return tsNode; @@ -108,17 +101,31 @@ function dummy() { ); } - #processSignalImports(signalImports: readonly string[]) { + #replaceSignalImport(method: FunctionDeclaration): Identifier { const { imports } = this.#dependencyManager; - signalImports.forEach((signalImport) => { - const result = imports.default.iter().find(([path]) => path.includes(signalImport)); + if (method.type) { + const type = traverse(method.type, (node) => + ts.isIdentifier(node) && signals.includes(node.text) ? node : undefined, + ); + + if (type) { + const signalId = imports.named.getIdentifier(HILLA_REACT_SIGNALS, type.text); - if (result) { - const [path, id] = result; - imports.default.remove(path); - imports.named.add(HILLA_REACT_SIGNALS, id.text, true, id); + if (signalId) { + return signalId; + } + + const result = imports.default.iter().find(([_p, id]) => id.text === type.text); + + if (result) { + const [path] = result; + imports.default.remove(path); + return imports.named.add(HILLA_REACT_SIGNALS, type.text, false, type); + } } - }); + } + + throw new Error('Signal type not found'); } } diff --git a/packages/ts/generator-plugin-signals/test/fixtures/NumberSignalServiceMix.snap.ts b/packages/ts/generator-plugin-signals/test/fixtures/NumberSignalServiceMix.snap.ts index ffdf1de04f..3e1d8847f5 100644 --- a/packages/ts/generator-plugin-signals/test/fixtures/NumberSignalServiceMix.snap.ts +++ b/packages/ts/generator-plugin-signals/test/fixtures/NumberSignalServiceMix.snap.ts @@ -1,7 +1,15 @@ import { EndpointRequestInit as EndpointRequestInit_1 } from "@vaadin/hilla-frontend"; -import { type NumberSignal as NumberSignal_1, NumberSignalChannel as NumberSignalChannel_1 } from "@vaadin/hilla-react-signals"; +import { createSignalChannel as createSignalChannel_1, NumberSignal as NumberSignal_1 } from "@vaadin/hilla-react-signals"; import client_1 from "./connect-client.default.js"; -function counter_1(): NumberSignal_1 { return new NumberSignalChannel_1("NumberSignalService.counter", client_1).signal; } +function counter_1(): NumberSignal_1 { + const signal = new NumberSignal_1(); + createSignalChannel_1(signal, "NumberSignalService.counter", client_1); + return signal; +} async function sayHello_1(name: string, init?: EndpointRequestInit_1): Promise { return client_1.call("NumberSignalService", "sayHello", { name }, init); } -function sharedValue_1(): NumberSignal_1 { return new NumberSignalChannel_1("NumberSignalService.sharedValue", client_1).signal; } +function sharedValue_1(): NumberSignal_1 { + const signal = new NumberSignal_1(); + createSignalChannel_1(signal, "NumberSignalService.sharedValue", client_1); + return signal; +} export { counter_1 as counter, sayHello_1 as sayHello, sharedValue_1 as sharedValue }; diff --git a/packages/ts/generator-plugin-signals/test/fixtures/NumberSignalServiceSignalOnly.snap.ts b/packages/ts/generator-plugin-signals/test/fixtures/NumberSignalServiceSignalOnly.snap.ts index 923598556d..ef26db2847 100644 --- a/packages/ts/generator-plugin-signals/test/fixtures/NumberSignalServiceSignalOnly.snap.ts +++ b/packages/ts/generator-plugin-signals/test/fixtures/NumberSignalServiceSignalOnly.snap.ts @@ -1,5 +1,13 @@ -import { type NumberSignal as NumberSignal_1, NumberSignalChannel as NumberSignalChannel_1 } from "@vaadin/hilla-react-signals"; +import { createSignalChannel as createSignalChannel_1, NumberSignal as NumberSignal_1 } from "@vaadin/hilla-react-signals"; import client_1 from "./connect-client.default.js"; -function counter_1(): NumberSignal_1 { return new NumberSignalChannel_1("NumberSignalService.counter", client_1).signal; } -function sharedValue_1(): NumberSignal_1 { return new NumberSignalChannel_1("NumberSignalService.sharedValue", client_1).signal; } +function counter_1(): NumberSignal_1 { + const signal = new NumberSignal_1(); + createSignalChannel_1(signal, "NumberSignalService.counter", client_1); + return signal; +} +function sharedValue_1(): NumberSignal_1 { + const signal = new NumberSignal_1(); + createSignalChannel_1(signal, "NumberSignalService.sharedValue", client_1); + return signal; +} export { counter_1 as counter, sharedValue_1 as sharedValue }; From 7016595f5ddcdfb49120d8f0c7276a72bd658ec2 Mon Sep 17 00:00:00 2001 From: Vlad Rindevich Date: Wed, 14 Aug 2024 19:47:51 +0300 Subject: [PATCH 05/22] refactor(react-signals): re-implement channel & signal implementation to support multi-subscription --- .../ts/react-signals/src/SignalChannel.ts | 126 +++++++++++------- packages/ts/react-signals/src/Signals.ts | 44 +++++- packages/ts/react-signals/src/index.ts | 2 +- .../react-signals/test/SignalChannel.spec.tsx | 34 ++--- 4 files changed, 140 insertions(+), 66 deletions(-) diff --git a/packages/ts/react-signals/src/SignalChannel.ts b/packages/ts/react-signals/src/SignalChannel.ts index b382ba0fa9..810365b3e3 100644 --- a/packages/ts/react-signals/src/SignalChannel.ts +++ b/packages/ts/react-signals/src/SignalChannel.ts @@ -7,7 +7,7 @@ const ENDPOINT = 'SignalsHandler'; /** * Types of changes that can be produced or processed by a signal. */ -export enum StateChangeType { +export enum StateEventType { SET = 'set', SNAPSHOT = 'snapshot', } @@ -15,65 +15,97 @@ export enum StateChangeType { /** * An object that describes the change of the signal state. */ -export type StateChange = Readonly<{ +export type StateEvent = Readonly<{ id: string; - type: StateChangeType; - value: T; + type: StateEventType; + value: unknown; }>; -export type Value> = S extends ValueSignal ? T : never; +/** + * A simple timeout helper class. + */ +class Timeout { + readonly #timeout: number; + readonly #callback: () => void; + #timeoutId?: ReturnType; -export type SignalChannel> = Readonly<{ - id: string; - subscription: Subscription>>; -}>; + constructor(callback: () => void, timeout: number) { + this.#callback = callback; + this.#timeout = timeout; + } + + restart() { + if (this.#timeoutId) { + clearTimeout(this.#timeoutId); + } + + this.#timeoutId = setTimeout(this.#callback, this.#timeout); + } +} /** - * Creates a signal channel that can be used to communicate with a server-side. + * A signal channel that can be used to communicate with a server-side. * * The signal channel is responsible for subscribing to the server-side signal * and updating the local signal based on the received events. * * @typeParam S - The type of the signal instance. - * @param signal - The local signal instance to be updated. - * @param signalProviderEndpointMethod - The method name of the signal provider - * service. - * @param client - The client instance to be used for communication. - * @returns The id of the created signal channel. */ -export function createSignalChannel>( - signal: S, - signalProviderEndpointMethod: string, - client: ConnectClient, -): SignalChannel { - const id = nanoid(); +export class SignalChannel { + readonly #id = nanoid(); + readonly #timeout: Timeout; + readonly #client: ConnectClient; + readonly #method: string; + #subscription?: Subscription; - let paused = false; - signal.subscribe((value) => { - if (!paused && value) { - client - .call(ENDPOINT, 'update', { - clientSignalId: id, - change: { id: nanoid(), type: StateChangeType.SET, value }, - }) - .catch((error) => { - throw error; - }); - } - }); + /** + * @param client - The client instance to be used for communication. + * @param signalProviderEndpointMethod - The method name of the signal provider + * service. + * @param timeout - The timeout in milliseconds after which the connection + * will be closed. The timeout is restarted every time a new signal is + * connected or a signal value is updated. Default is 2000 ms. + * + * @returns The signal channel instance. + */ + constructor(client: ConnectClient, signalProviderEndpointMethod: string, timeout = 2000) { + this.#client = client; + this.#method = signalProviderEndpointMethod; + this.#timeout = new Timeout(() => this.#subscription?.cancel(), timeout); + } - const subscription: Subscription>> = client - .subscribe(ENDPOINT, 'subscribe', { - signalProviderEndpointMethod, - clientSignalId: id, - }) - .onNext((change: StateChange>) => { - if (change.type === StateChangeType.SNAPSHOT) { - paused = true; - signal.value = change.value; - paused = false; - } - }); + get id(): string { + return this.#id; + } + + /** + * Connects the local signal to the server. + * + * @param signal - The signal instance to be connected. + */ + connect(signal: ValueSignal): void { + this.#timeout.restart(); - return { id, subscription }; + this.#subscription ??= this.#client + .subscribe(ENDPOINT, 'subscribe', { + signalProviderEndpointMethod: this.#method, + clientSignalId: this.#id, + }) + .onNext((event: StateEvent) => { + if (event.type === StateEventType.SNAPSHOT) { + signal.value = event.value; + } + }); + + signal.subscribe((value, done) => { + this.#timeout.restart(); + + done( + this.#client.call(ENDPOINT, 'update', { + clientSignalId: this.#id, + event: { id: nanoid(), type: StateEventType.SET, value }, + }), + ); + }); + } } diff --git a/packages/ts/react-signals/src/Signals.ts b/packages/ts/react-signals/src/Signals.ts index ff9bac2a39..6b16d1555d 100644 --- a/packages/ts/react-signals/src/Signals.ts +++ b/packages/ts/react-signals/src/Signals.ts @@ -1,4 +1,10 @@ +import { signal } from '@preact/signals-react'; import { Signal } from './core.js'; +import type { SignalChannel } from './SignalChannel.js'; + +export type ValueSignalOptions = Readonly<{ + channel?: SignalChannel; +}>; /** * A signal that holds a value. The underlying @@ -7,7 +13,43 @@ import { Signal } from './core.js'; * * @internal */ -export abstract class ValueSignal extends Signal {} +export class ValueSignal extends Signal { + readonly #pending = signal(false); + readonly #error = signal(undefined); + + constructor(value?: T, options?: ValueSignalOptions) { + super(value); + options?.channel?.connect(this); + } + + /** + * Defines whether the signal is currently pending server-side response. + */ + get pending(): boolean { + return this.#pending.value; + } + + /** + * Defines whether the signal has an error. + */ + get error(): Error | undefined { + return this.#error.value; + } + + override subscribe(connector: (value: T, done: (promise: Promise) => void) => void): () => void { + return super.subscribe((value) => { + connector(value, (promise) => { + promise + .catch((error: unknown) => { + this.#error.value = error instanceof Error ? error : new Error(String(error)); + }) + .finally(() => { + this.#pending.value = false; + }); + }); + }); + } +} /** * A signal that holds a number value. The underlying diff --git a/packages/ts/react-signals/src/index.ts b/packages/ts/react-signals/src/index.ts index 3da9f3af0a..5a81d109af 100644 --- a/packages/ts/react-signals/src/index.ts +++ b/packages/ts/react-signals/src/index.ts @@ -1,4 +1,4 @@ // eslint-disable-next-line import/export export * from './core.js'; -export { createSignalChannel, type SignalChannel } from './SignalChannel.js'; +export { SignalChannel } from './SignalChannel.js'; export { NumberSignal, ValueSignal } from './Signals.js'; diff --git a/packages/ts/react-signals/test/SignalChannel.spec.tsx b/packages/ts/react-signals/test/SignalChannel.spec.tsx index 847b19020c..e79bc1e8ff 100644 --- a/packages/ts/react-signals/test/SignalChannel.spec.tsx +++ b/packages/ts/react-signals/test/SignalChannel.spec.tsx @@ -4,8 +4,8 @@ import { render } from '@testing-library/react'; import { ConnectClient, type Subscription } from '@vaadin/hilla-frontend'; import sinon from 'sinon'; import sinonChai from 'sinon-chai'; -import { NumberSignal, type SignalChannel } from '../src/index.js'; -import { createSignalChannel, type StateChange, StateChangeType } from '../src/SignalChannel.js'; +import { NumberSignal, SignalChannel } from '../src/index.js'; +import { StateEventType, type StateEvent } from '../src/SignalChannel.js'; import { nextFrame } from './utils.js'; use(sinonChai); @@ -13,24 +13,24 @@ use(sinonChai); describe('@vaadin/hilla-react-signals', () => { describe('SignalChannel', () => { function simulateReceivedChange( - connectSubscriptionMock: sinon.SinonSpiedInstance>>, - change: StateChange, + connectSubscriptionMock: sinon.SinonSpiedInstance>, + event: StateEvent, ) { const onNextCallback = connectSubscriptionMock.onNext.getCall(0).args[0]; // eslint-disable-next-line @typescript-eslint/no-unsafe-call - onNextCallback(change); + onNextCallback(event); } let connectClientMock: sinon.SinonStubbedInstance; - let connectSubscriptionMock: sinon.SinonSpiedInstance>>; + let connectSubscriptionMock: sinon.SinonSpiedInstance>; let signal: NumberSignal; - let channel: SignalChannel; + let channel: SignalChannel; beforeEach(() => { connectClientMock = sinon.createStubInstance(ConnectClient); connectClientMock.call.resolves(); - connectSubscriptionMock = sinon.spy>>({ + connectSubscriptionMock = sinon.spy>({ cancel() {}, context() { return this; @@ -48,8 +48,8 @@ describe('@vaadin/hilla-react-signals', () => { // Mock the subscribe method connectClientMock.subscribe.returns(connectSubscriptionMock); - signal = new NumberSignal(); - channel = createSignalChannel(signal, 'testEndpoint', connectClientMock); + channel = new SignalChannel(connectClientMock, 'testEndpoint'); + signal = new NumberSignal(undefined, { channel }); connectClientMock.call.resetHistory(); }); @@ -76,7 +76,7 @@ describe('@vaadin/hilla-react-signals', () => { expect(connectClientMock.call).to.be.have.been.calledOnce; expect(connectClientMock.call).to.have.been.calledWithMatch('SignalsHandler', 'update', { clientSignalId: channel.id, - change: { type: StateChangeType.SET, value: 42 }, + event: { type: StateEventType.SET, value: 42 }, }); }); @@ -84,7 +84,7 @@ describe('@vaadin/hilla-react-signals', () => { expect(signal.value).to.be.undefined; // Simulate the event received from the server: - const snapshotEvent: StateChange = { id: 'someId', type: StateChangeType.SNAPSHOT, value: 42 }; + const snapshotEvent: StateEvent = { id: 'someId', type: StateEventType.SNAPSHOT, value: 42 }; simulateReceivedChange(connectSubscriptionMock, snapshotEvent); // Check if the signal value is updated: @@ -93,13 +93,13 @@ describe('@vaadin/hilla-react-signals', () => { it('should render the updated value', async () => { const numberSignal = signal; - simulateReceivedChange(connectSubscriptionMock, { id: 'someId', type: StateChangeType.SNAPSHOT, value: 42 }); + simulateReceivedChange(connectSubscriptionMock, { id: 'someId', type: StateEventType.SNAPSHOT, value: 42 }); const result = render(Value is {numberSignal}); await nextFrame(); expect(result.container.textContent).to.equal('Value is 42'); - simulateReceivedChange(connectSubscriptionMock, { id: 'someId', type: StateChangeType.SNAPSHOT, value: 99 }); + simulateReceivedChange(connectSubscriptionMock, { id: 'someId', type: StateEventType.SNAPSHOT, value: 99 }); await nextFrame(); expect(result.container.textContent).to.equal('Value is 99'); }); @@ -116,7 +116,7 @@ describe('@vaadin/hilla-react-signals', () => { signal.value = 42; expect(connectClientMock.call).to.have.been.calledOnce; expect(connectClientMock.call).to.have.been.calledWithMatch('SignalsHandler', 'update', { - change: { type: StateChangeType.SET, value: 42 }, + event: { type: StateEventType.SET, value: 42 }, }); signal.value = 0; @@ -126,11 +126,11 @@ describe('@vaadin/hilla-react-signals', () => { signal.value += 1; expect(connectClientMock.call).to.have.been.calledOnce; expect(connectClientMock.call).to.have.been.calledWithMatch('SignalsHandler', 'update', { - change: { type: StateChangeType.SET, value: 1 }, + event: { type: StateEventType.SET, value: 1 }, }); }); - it('should throw an error when the server call fails', async () => { + it('should throw an error when the server call fails', () => { connectClientMock.call.rejects(new Error('Server error')); signal.value = 42; }); From 53821254a515f694b7948030706e7db6967026f5 Mon Sep 17 00:00:00 2001 From: Vlad Rindevich Date: Wed, 14 Aug 2024 20:06:01 +0300 Subject: [PATCH 06/22] refactor(generator-plugin-signals): re-implement channel generation --- .../src/SignalProcessor.ts | 39 +++++++------------ .../fixtures/NumberSignalServiceMix.snap.ts | 24 ++++++------ .../NumberSignalServiceSignalOnly.snap.ts | 18 ++++----- 3 files changed, 31 insertions(+), 50 deletions(-) diff --git a/packages/ts/generator-plugin-signals/src/SignalProcessor.ts b/packages/ts/generator-plugin-signals/src/SignalProcessor.ts index ab92cad66b..f1db97cc33 100644 --- a/packages/ts/generator-plugin-signals/src/SignalProcessor.ts +++ b/packages/ts/generator-plugin-signals/src/SignalProcessor.ts @@ -1,17 +1,19 @@ import type Plugin from '@vaadin/hilla-generator-core/Plugin.js'; import { template, transform, traverse } from '@vaadin/hilla-generator-utils/ast.js'; +import createFullyUniqueIdentifier from '@vaadin/hilla-generator-utils/createFullyUniqueIdentifier.js'; import createSourceFile from '@vaadin/hilla-generator-utils/createSourceFile.js'; import DependencyManager from '@vaadin/hilla-generator-utils/dependencies/DependencyManager.js'; import PathManager from '@vaadin/hilla-generator-utils/dependencies/PathManager.js'; -import ts, { type FunctionDeclaration, type Identifier, type SourceFile, type TypeNode } from 'typescript'; +import ts, { type FunctionDeclaration, type Identifier, type SourceFile } from 'typescript'; const HILLA_REACT_SIGNALS = '@vaadin/hilla-react-signals'; -const CREATE_SIGNAL_CHANNEL = '$CREATE_SIGNAL_CHANNEL$'; +const SIGNAL_CHANNEL = '$SIGNAL_CHANNEL$'; const CONNECT_CLIENT = '$CONNECT_CLIENT$'; +const METHOD_NAME = '$METHOD_NAME$'; const SIGNAL = '$SIGNAL$'; -const signals = ['NumberSignal', 'ValueSignal']; +const signals = ['NumberSignal', 'RemoteSignal']; export default class SignalProcessor { readonly #dependencyManager: DependencyManager; @@ -33,7 +35,7 @@ export default class SignalProcessor { this.#owner.logger.debug(`Processing signals: ${this.#service}`); const { imports } = this.#dependencyManager; - const createSignalChannelId = imports.named.add(HILLA_REACT_SIGNALS, 'createSignalChannel'); + const signalChannelId = imports.named.add(HILLA_REACT_SIGNALS, 'SignalChannel'); const [, connectClientId] = imports.default.iter().find(([path]) => path.includes('connect-client'))!; @@ -43,35 +45,20 @@ export default class SignalProcessor { const [file] = ts.transform(this.#sourceFile, [ transform((tsNode) => { if (ts.isFunctionDeclaration(tsNode) && tsNode.name && this.#methods.has(tsNode.name.text)) { - const methodName = tsNode.name.text; const signalId = this.#replaceSignalImport(tsNode); - const body = template( - ` -function dummy() { - const signal = new ${SIGNAL}(); - ${CREATE_SIGNAL_CHANNEL}(signal, '${this.#service}.${methodName}', ${CONNECT_CLIENT}); - return signal; -}`, - (statements) => (statements[0] as FunctionDeclaration).body?.statements, + return template( + `(function ${METHOD_NAME}(this: ${SIGNAL_CHANNEL}) { + return new ${SIGNAL}(undefined, { channel: this }); +}).bind(new ${SIGNAL_CHANNEL}(${CONNECT_CLIENT}, '${this.#service}.${tsNode.name.text}'));`, + (statements) => statements, [ - transform((node) => - ts.isIdentifier(node) && node.text === CREATE_SIGNAL_CHANNEL ? createSignalChannelId : node, - ), + transform((node) => (ts.isIdentifier(node) && node.text === METHOD_NAME ? tsNode.name : node)), + transform((node) => (ts.isIdentifier(node) && node.text === SIGNAL_CHANNEL ? signalChannelId : node)), transform((node) => (ts.isIdentifier(node) && node.text === SIGNAL ? signalId : node)), transform((node) => (ts.isIdentifier(node) && node.text === CONNECT_CLIENT ? connectClientId : node)), ], ); - - return ts.factory.createFunctionDeclaration( - tsNode.modifiers?.filter((modifier) => modifier.kind !== ts.SyntaxKind.AsyncKeyword), - tsNode.asteriskToken, - tsNode.name, - tsNode.typeParameters, - tsNode.parameters.filter(({ name }) => !(ts.isIdentifier(name) && name.text === 'init')), - ts.factory.createTypeReferenceNode(signalId), - ts.factory.createBlock(body ?? [], true), - ); } return tsNode; }), diff --git a/packages/ts/generator-plugin-signals/test/fixtures/NumberSignalServiceMix.snap.ts b/packages/ts/generator-plugin-signals/test/fixtures/NumberSignalServiceMix.snap.ts index 3e1d8847f5..c298f98b53 100644 --- a/packages/ts/generator-plugin-signals/test/fixtures/NumberSignalServiceMix.snap.ts +++ b/packages/ts/generator-plugin-signals/test/fixtures/NumberSignalServiceMix.snap.ts @@ -1,15 +1,13 @@ -import { EndpointRequestInit as EndpointRequestInit_1 } from "@vaadin/hilla-frontend"; -import { createSignalChannel as createSignalChannel_1, NumberSignal as NumberSignal_1 } from "@vaadin/hilla-react-signals"; -import client_1 from "./connect-client.default.js"; -function counter_1(): NumberSignal_1 { - const signal = new NumberSignal_1(); - createSignalChannel_1(signal, "NumberSignalService.counter", client_1); - return signal; -} -async function sayHello_1(name: string, init?: EndpointRequestInit_1): Promise { return client_1.call("NumberSignalService", "sayHello", { name }, init); } -function sharedValue_1(): NumberSignal_1 { - const signal = new NumberSignal_1(); - createSignalChannel_1(signal, "NumberSignalService.sharedValue", client_1); - return signal; +import { EndpointRequestInit as EndpointRequestInit_1 } from '@vaadin/hilla-frontend'; +import { NumberSignal as NumberSignal_1, SignalChannel as SignalChannel_1 } from '@vaadin/hilla-react-signals'; +import client_1 from './connect-client.default.js'; +(function counter_1(this: SignalChannel_1) { + return new NumberSignal_1(undefined, { channel: this }); +}).bind(new SignalChannel_1(client_1, 'NumberSignalService.counter')); +async function sayHello_1(name: string, init?: EndpointRequestInit_1): Promise { + return client_1.call('NumberSignalService', 'sayHello', { name }, init); } +(function sharedValue_1(this: SignalChannel_1) { + return new NumberSignal_1(undefined, { channel: this }); +}).bind(new SignalChannel_1(client_1, 'NumberSignalService.sharedValue')); export { counter_1 as counter, sayHello_1 as sayHello, sharedValue_1 as sharedValue }; diff --git a/packages/ts/generator-plugin-signals/test/fixtures/NumberSignalServiceSignalOnly.snap.ts b/packages/ts/generator-plugin-signals/test/fixtures/NumberSignalServiceSignalOnly.snap.ts index ef26db2847..97c1629288 100644 --- a/packages/ts/generator-plugin-signals/test/fixtures/NumberSignalServiceSignalOnly.snap.ts +++ b/packages/ts/generator-plugin-signals/test/fixtures/NumberSignalServiceSignalOnly.snap.ts @@ -1,13 +1,9 @@ -import { createSignalChannel as createSignalChannel_1, NumberSignal as NumberSignal_1 } from "@vaadin/hilla-react-signals"; +import { NumberSignal as NumberSignal_1, SignalChannel as SignalChannel_1 } from "@vaadin/hilla-react-signals"; import client_1 from "./connect-client.default.js"; -function counter_1(): NumberSignal_1 { - const signal = new NumberSignal_1(); - createSignalChannel_1(signal, "NumberSignalService.counter", client_1); - return signal; -} -function sharedValue_1(): NumberSignal_1 { - const signal = new NumberSignal_1(); - createSignalChannel_1(signal, "NumberSignalService.sharedValue", client_1); - return signal; -} +(function counter_1(this: SignalChannel_1) { + return new NumberSignal_1(undefined, { channel: this }); +}).bind(new SignalChannel_1(client_1, "NumberSignalService.counter")); +(function sharedValue_1(this: SignalChannel_1) { + return new NumberSignal_1(undefined, { channel: this }); +}).bind(new SignalChannel_1(client_1, "NumberSignalService.sharedValue")); export { counter_1 as counter, sharedValue_1 as sharedValue }; From 8f185db008165dd20f614625d8b1681f30aa081d Mon Sep 17 00:00:00 2001 From: Vlad Rindevich Date: Thu, 15 Aug 2024 10:45:52 +0300 Subject: [PATCH 07/22] refactor(react-signals): remove timeout --- .../ts/react-signals/src/SignalChannel.ts | 38 +++---------------- 1 file changed, 5 insertions(+), 33 deletions(-) diff --git a/packages/ts/react-signals/src/SignalChannel.ts b/packages/ts/react-signals/src/SignalChannel.ts index 810365b3e3..27c4aecd48 100644 --- a/packages/ts/react-signals/src/SignalChannel.ts +++ b/packages/ts/react-signals/src/SignalChannel.ts @@ -21,28 +21,6 @@ export type StateEvent = Readonly<{ value: unknown; }>; -/** - * A simple timeout helper class. - */ -class Timeout { - readonly #timeout: number; - readonly #callback: () => void; - #timeoutId?: ReturnType; - - constructor(callback: () => void, timeout: number) { - this.#callback = callback; - this.#timeout = timeout; - } - - restart() { - if (this.#timeoutId) { - clearTimeout(this.#timeoutId); - } - - this.#timeoutId = setTimeout(this.#callback, this.#timeout); - } -} - /** * A signal channel that can be used to communicate with a server-side. * @@ -53,7 +31,6 @@ class Timeout { */ export class SignalChannel { readonly #id = nanoid(); - readonly #timeout: Timeout; readonly #client: ConnectClient; readonly #method: string; #subscription?: Subscription; @@ -62,30 +39,27 @@ export class SignalChannel { * @param client - The client instance to be used for communication. * @param signalProviderEndpointMethod - The method name of the signal provider * service. - * @param timeout - The timeout in milliseconds after which the connection - * will be closed. The timeout is restarted every time a new signal is - * connected or a signal value is updated. Default is 2000 ms. - * * @returns The signal channel instance. */ - constructor(client: ConnectClient, signalProviderEndpointMethod: string, timeout = 2000) { + constructor(client: ConnectClient, signalProviderEndpointMethod: string) { this.#client = client; this.#method = signalProviderEndpointMethod; - this.#timeout = new Timeout(() => this.#subscription?.cancel(), timeout); } get id(): string { return this.#id; } + cancel(): void { + this.#subscription?.cancel(); + } + /** * Connects the local signal to the server. * * @param signal - The signal instance to be connected. */ connect(signal: ValueSignal): void { - this.#timeout.restart(); - this.#subscription ??= this.#client .subscribe(ENDPOINT, 'subscribe', { signalProviderEndpointMethod: this.#method, @@ -98,8 +72,6 @@ export class SignalChannel { }); signal.subscribe((value, done) => { - this.#timeout.restart(); - done( this.#client.call(ENDPOINT, 'update', { clientSignalId: this.#id, From 941343ce746ad4751efd6c661cf9118b09a382bd Mon Sep 17 00:00:00 2001 From: Vlad Rindevich Date: Thu, 15 Aug 2024 10:54:51 +0300 Subject: [PATCH 08/22] refactor(react-signals): finalize channel API --- .../ts/react-signals/src/SignalChannel.ts | 25 +++++++++++++------ packages/ts/react-signals/src/Signals.ts | 25 ++++++++----------- 2 files changed, 27 insertions(+), 23 deletions(-) diff --git a/packages/ts/react-signals/src/SignalChannel.ts b/packages/ts/react-signals/src/SignalChannel.ts index 27c4aecd48..e63e931f79 100644 --- a/packages/ts/react-signals/src/SignalChannel.ts +++ b/packages/ts/react-signals/src/SignalChannel.ts @@ -1,5 +1,6 @@ import type { ConnectClient, Subscription } from '@vaadin/hilla-frontend'; import { nanoid } from 'nanoid'; +import { effect } from './core.js'; import type { ValueSignal } from './Signals.js'; const ENDPOINT = 'SignalsHandler'; @@ -52,14 +53,18 @@ export class SignalChannel { cancel(): void { this.#subscription?.cancel(); + this.#subscription = undefined; } /** * Connects the local signal to the server. * * @param signal - The signal instance to be connected. + * @param onUpdate - The callback that will be called when the signal is + * updated. */ - connect(signal: ValueSignal): void { + connect(signal: ValueSignal, onUpdate: (promise: Promise) => void): void { + let paused = false; this.#subscription ??= this.#client .subscribe(ENDPOINT, 'subscribe', { signalProviderEndpointMethod: this.#method, @@ -67,17 +72,21 @@ export class SignalChannel { }) .onNext((event: StateEvent) => { if (event.type === StateEventType.SNAPSHOT) { + paused = true; signal.value = event.value; + paused = false; } }); - signal.subscribe((value, done) => { - done( - this.#client.call(ENDPOINT, 'update', { - clientSignalId: this.#id, - event: { id: nanoid(), type: StateEventType.SET, value }, - }), - ); + signal.subscribe((value) => { + if (!paused) { + onUpdate( + this.#client.call(ENDPOINT, 'update', { + clientSignalId: this.#id, + event: { id: nanoid(), type: StateEventType.SET, value }, + }), + ); + } }); } } diff --git a/packages/ts/react-signals/src/Signals.ts b/packages/ts/react-signals/src/Signals.ts index 6b16d1555d..767f8bfc65 100644 --- a/packages/ts/react-signals/src/Signals.ts +++ b/packages/ts/react-signals/src/Signals.ts @@ -19,7 +19,16 @@ export class ValueSignal extends Signal { constructor(value?: T, options?: ValueSignalOptions) { super(value); - options?.channel?.connect(this); + options?.channel?.connect(this, (promise) => { + this.#pending.value = true; + promise + .catch((error: unknown) => { + this.#error.value = error instanceof Error ? error : new Error(String(error)); + }) + .finally(() => { + this.#pending.value = false; + }); + }); } /** @@ -35,20 +44,6 @@ export class ValueSignal extends Signal { get error(): Error | undefined { return this.#error.value; } - - override subscribe(connector: (value: T, done: (promise: Promise) => void) => void): () => void { - return super.subscribe((value) => { - connector(value, (promise) => { - promise - .catch((error: unknown) => { - this.#error.value = error instanceof Error ? error : new Error(String(error)); - }) - .finally(() => { - this.#pending.value = false; - }); - }); - }); - } } /** From e79c8900786740c1a68552fe2b0b9012964daa34 Mon Sep 17 00:00:00 2001 From: Vlad Rindevich Date: Thu, 15 Aug 2024 12:05:20 +0300 Subject: [PATCH 09/22] refactor(generator-plugin-signals): improve channel generation --- .../src/SignalProcessor.ts | 10 ++++++--- .../fixtures/NumberSignalServiceMix.snap.ts | 22 +++++++++---------- 2 files changed, 18 insertions(+), 14 deletions(-) diff --git a/packages/ts/generator-plugin-signals/src/SignalProcessor.ts b/packages/ts/generator-plugin-signals/src/SignalProcessor.ts index f1db97cc33..913ad54a42 100644 --- a/packages/ts/generator-plugin-signals/src/SignalProcessor.ts +++ b/packages/ts/generator-plugin-signals/src/SignalProcessor.ts @@ -8,6 +8,7 @@ import ts, { type FunctionDeclaration, type Identifier, type SourceFile } from ' const HILLA_REACT_SIGNALS = '@vaadin/hilla-react-signals'; +const CHANNEL = '$CHANNEL$'; const SIGNAL_CHANNEL = '$SIGNAL_CHANNEL$'; const CONNECT_CLIENT = '$CONNECT_CLIENT$'; const METHOD_NAME = '$METHOD_NAME$'; @@ -46,13 +47,16 @@ export default class SignalProcessor { transform((tsNode) => { if (ts.isFunctionDeclaration(tsNode) && tsNode.name && this.#methods.has(tsNode.name.text)) { const signalId = this.#replaceSignalImport(tsNode); + const channel = createFullyUniqueIdentifier('channel'); return template( - `(function ${METHOD_NAME}(this: ${SIGNAL_CHANNEL}) { - return new ${SIGNAL}(undefined, { channel: this }); -}).bind(new ${SIGNAL_CHANNEL}(${CONNECT_CLIENT}, '${this.#service}.${tsNode.name.text}'));`, + `const ${CHANNEL} = new ${SIGNAL_CHANNEL}(${CONNECT_CLIENT}, '${this.#service}.${tsNode.name.text}') +function ${METHOD_NAME}() { + return new ${SIGNAL}(undefined, { channel: ${CHANNEL} }); +}`, (statements) => statements, [ + transform((node) => (ts.isIdentifier(node) && node.text === CHANNEL ? channel : node)), transform((node) => (ts.isIdentifier(node) && node.text === METHOD_NAME ? tsNode.name : node)), transform((node) => (ts.isIdentifier(node) && node.text === SIGNAL_CHANNEL ? signalChannelId : node)), transform((node) => (ts.isIdentifier(node) && node.text === SIGNAL ? signalId : node)), diff --git a/packages/ts/generator-plugin-signals/test/fixtures/NumberSignalServiceMix.snap.ts b/packages/ts/generator-plugin-signals/test/fixtures/NumberSignalServiceMix.snap.ts index c298f98b53..65d0338ea2 100644 --- a/packages/ts/generator-plugin-signals/test/fixtures/NumberSignalServiceMix.snap.ts +++ b/packages/ts/generator-plugin-signals/test/fixtures/NumberSignalServiceMix.snap.ts @@ -1,13 +1,13 @@ -import { EndpointRequestInit as EndpointRequestInit_1 } from '@vaadin/hilla-frontend'; -import { NumberSignal as NumberSignal_1, SignalChannel as SignalChannel_1 } from '@vaadin/hilla-react-signals'; -import client_1 from './connect-client.default.js'; -(function counter_1(this: SignalChannel_1) { - return new NumberSignal_1(undefined, { channel: this }); -}).bind(new SignalChannel_1(client_1, 'NumberSignalService.counter')); -async function sayHello_1(name: string, init?: EndpointRequestInit_1): Promise { - return client_1.call('NumberSignalService', 'sayHello', { name }, init); +import { EndpointRequestInit as EndpointRequestInit_1 } from "@vaadin/hilla-frontend"; +import { NumberSignal as NumberSignal_1, SignalChannel as SignalChannel_1 } from "@vaadin/hilla-react-signals"; +import client_1 from "./connect-client.default.js"; +const channel_1 = new SignalChannel_1(client_1, "NumberSignalService.counter"); +function counter_1() { + return new NumberSignal_1(undefined, { channel: channel_1 }); +} +async function sayHello_1(name: string, init?: EndpointRequestInit_1): Promise { return client_1.call("NumberSignalService", "sayHello", { name }, init); } +const channel_2 = new SignalChannel_1(client_1, "NumberSignalService.sharedValue"); +function sharedValue_1() { + return new NumberSignal_1(undefined, { channel: channel_2 }); } -(function sharedValue_1(this: SignalChannel_1) { - return new NumberSignal_1(undefined, { channel: this }); -}).bind(new SignalChannel_1(client_1, 'NumberSignalService.sharedValue')); export { counter_1 as counter, sayHello_1 as sayHello, sharedValue_1 as sharedValue }; From 3b1142bd5d1282a951a4f740dc55503564b20eba Mon Sep 17 00:00:00 2001 From: Vlad Rindevich Date: Thu, 15 Aug 2024 12:05:53 +0300 Subject: [PATCH 10/22] test(generator-plugin-signals): update snapshot --- .../fixtures/NumberSignalServiceSignalOnly.snap.ts | 14 ++++++++------ packages/ts/react-signals/src/SignalChannel.ts | 1 - 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/packages/ts/generator-plugin-signals/test/fixtures/NumberSignalServiceSignalOnly.snap.ts b/packages/ts/generator-plugin-signals/test/fixtures/NumberSignalServiceSignalOnly.snap.ts index 97c1629288..48a11b0ed8 100644 --- a/packages/ts/generator-plugin-signals/test/fixtures/NumberSignalServiceSignalOnly.snap.ts +++ b/packages/ts/generator-plugin-signals/test/fixtures/NumberSignalServiceSignalOnly.snap.ts @@ -1,9 +1,11 @@ import { NumberSignal as NumberSignal_1, SignalChannel as SignalChannel_1 } from "@vaadin/hilla-react-signals"; import client_1 from "./connect-client.default.js"; -(function counter_1(this: SignalChannel_1) { - return new NumberSignal_1(undefined, { channel: this }); -}).bind(new SignalChannel_1(client_1, "NumberSignalService.counter")); -(function sharedValue_1(this: SignalChannel_1) { - return new NumberSignal_1(undefined, { channel: this }); -}).bind(new SignalChannel_1(client_1, "NumberSignalService.sharedValue")); +const channel_1 = new SignalChannel_1(client_1, "NumberSignalService.counter"); +function counter_1() { + return new NumberSignal_1(undefined, { channel: channel_1 }); +} +const channel_2 = new SignalChannel_1(client_1, "NumberSignalService.sharedValue"); +function sharedValue_1() { + return new NumberSignal_1(undefined, { channel: channel_2 }); +} export { counter_1 as counter, sharedValue_1 as sharedValue }; diff --git a/packages/ts/react-signals/src/SignalChannel.ts b/packages/ts/react-signals/src/SignalChannel.ts index e63e931f79..9246d16f1d 100644 --- a/packages/ts/react-signals/src/SignalChannel.ts +++ b/packages/ts/react-signals/src/SignalChannel.ts @@ -1,6 +1,5 @@ import type { ConnectClient, Subscription } from '@vaadin/hilla-frontend'; import { nanoid } from 'nanoid'; -import { effect } from './core.js'; import type { ValueSignal } from './Signals.js'; const ENDPOINT = 'SignalsHandler'; From d047415f6b75281d3bbc3b49be11047b9e595eff Mon Sep 17 00:00:00 2001 From: Vlad Rindevich Date: Thu, 15 Aug 2024 13:36:15 +0300 Subject: [PATCH 11/22] refactor(react-signals): run update for all dependencies in a single batch --- packages/ts/react-signals/src/SignalChannel.ts | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/packages/ts/react-signals/src/SignalChannel.ts b/packages/ts/react-signals/src/SignalChannel.ts index 9246d16f1d..1e1be28018 100644 --- a/packages/ts/react-signals/src/SignalChannel.ts +++ b/packages/ts/react-signals/src/SignalChannel.ts @@ -1,5 +1,6 @@ import type { ConnectClient, Subscription } from '@vaadin/hilla-frontend'; import { nanoid } from 'nanoid'; +import { batch } from './core.js'; import type { ValueSignal } from './Signals.js'; const ENDPOINT = 'SignalsHandler'; @@ -33,6 +34,7 @@ export class SignalChannel { readonly #id = nanoid(); readonly #client: ConnectClient; readonly #method: string; + #dependencies: readonly ValueSignal[] = []; #subscription?: Subscription; /** @@ -64,6 +66,9 @@ export class SignalChannel { */ connect(signal: ValueSignal, onUpdate: (promise: Promise) => void): void { let paused = false; + + this.#dependencies = [...this.#dependencies, signal]; + this.#subscription ??= this.#client .subscribe(ENDPOINT, 'subscribe', { signalProviderEndpointMethod: this.#method, @@ -72,7 +77,11 @@ export class SignalChannel { .onNext((event: StateEvent) => { if (event.type === StateEventType.SNAPSHOT) { paused = true; - signal.value = event.value; + batch(() => { + for (const dependency of this.#dependencies) { + dependency.value = event.value; + } + }); paused = false; } }); From 00bf6f53ec375fb3d789fef78b4f71116799b9f7 Mon Sep 17 00:00:00 2001 From: Vlad Rindevich Date: Thu, 15 Aug 2024 17:13:38 +0300 Subject: [PATCH 12/22] refactor(react-signals): merge SignalChannel with Signal --- packages/ts/react-signals/.eslintrc | 1 + .../ts/react-signals/src/FullStackSignal.ts | 157 ++++++++++++++++++ .../ts/react-signals/src/SignalChannel.ts | 100 ----------- packages/ts/react-signals/src/Signals.ts | 47 +----- packages/ts/react-signals/src/index.ts | 4 +- ...nnel.spec.tsx => FullStackSignal.spec.tsx} | 68 ++++---- .../ts/react-signals/test/Signals.spec.tsx | 52 ++++-- 7 files changed, 235 insertions(+), 194 deletions(-) create mode 100644 packages/ts/react-signals/src/FullStackSignal.ts delete mode 100644 packages/ts/react-signals/src/SignalChannel.ts rename packages/ts/react-signals/test/{SignalChannel.spec.tsx => FullStackSignal.spec.tsx} (53%) diff --git a/packages/ts/react-signals/.eslintrc b/packages/ts/react-signals/.eslintrc index d445a4e4e1..6ac92d4ca9 100644 --- a/packages/ts/react-signals/.eslintrc +++ b/packages/ts/react-signals/.eslintrc @@ -1,4 +1,5 @@ { + "extends": ["../../../.eslintrc"], "parserOptions": { "project": "./tsconfig.json" } diff --git a/packages/ts/react-signals/src/FullStackSignal.ts b/packages/ts/react-signals/src/FullStackSignal.ts new file mode 100644 index 0000000000..bc9bc637f2 --- /dev/null +++ b/packages/ts/react-signals/src/FullStackSignal.ts @@ -0,0 +1,157 @@ +import { signal } from '@preact/signals-react'; +import type { ConnectClient, Subscription } from '@vaadin/hilla-frontend'; +import { nanoid } from 'nanoid'; +import { Signal } from './core.js'; + +const ENDPOINT = 'SignalsHandler'; + +type SubscriptionEvent = Readonly<{ + type: StateEventType; + value: T; +}>; + +/** + * Types of changes that can be produced or processed by a signal. + */ +export enum StateEventType { + SET = 'set', + SNAPSHOT = 'snapshot', +} + +/** + * An object that describes the change of the signal state. + */ +export type StateEvent = Readonly<{ + id: string; + type: StateEventType; + value: T; +}>; +/** + * Options for creating a full-stack signal. + */ +export type FullStackSignalOptions = Readonly<{ + /** + * The client instance to be used for communication. + */ + client: ConnectClient; + /** + * The method name of the signal provider service. + */ + method: string; +}>; + +class SubscriptionManager { + readonly #client; + readonly #method: string; + readonly #id: string; + #subscription?: Subscription>; + + constructor(client: ConnectClient, method: string, id: string) { + this.#client = client; + this.#method = method; + this.#id = id; + } + + get subscription() { + return this.#subscription; + } + + subscribe() { + this.#subscription ??= this.#client.subscribe(ENDPOINT, 'subscribe', { + signalProviderEndpointMethod: this.#method, + clientSignalId: this.#id, + }); + + return this.#subscription; + } + + async call(event: SubscriptionEvent): Promise { + await this.#client.call(ENDPOINT, 'update', { + clientSignalId: this.#id, + event, + }); + } + + cancel() { + this.#subscription?.cancel(); + this.#subscription = undefined; + } +} + +/** + * A signal that holds a shared value. Each change to the value is propagated to + * the server-side signal provider. At the same time, each change received from + * the server-side signal provider is propagated to the local signal and it's + * subscribers. + * + * @internal + */ +export abstract class FullStackSignal extends Signal { + readonly id = nanoid(); + readonly #pending = signal(false); + readonly #error = signal(undefined); + readonly #manager: SubscriptionManager; + + constructor(value: T | undefined, { client, method }: FullStackSignalOptions) { + super(value); + this.#manager = new SubscriptionManager(client, method, this.id); + + let paused = false; + this.#manager.subscribe().onNext((event: StateEvent) => { + if (event.type === StateEventType.SNAPSHOT) { + paused = true; + this.value = event.value; + paused = false; + } + }); + + this.subscribe((v) => { + if (!paused) { + this.#pending.value = true; + this.#manager + .call({ + type: StateEventType.SET, + value: v, + }) + .catch((error) => { + this.#error.value = error; + }) + .finally(() => { + this.#pending.value = false; + }); + } + }); + } + + /** + * Defines whether the signal is currently awaits a server-side response. + */ + get pending(): boolean { + return this.#pending.value; + } + + /** + * Defines whether the signal has an error. + */ + get error(): Error | undefined { + return this.#error.value; + } + + get updating(): Promise { + return new Promise((resolve) => { + const unsubscribe = this.#pending.subscribe((value) => { + if (value) { + resolve(); + unsubscribe(); + } + }); + }); + } + + /** + * Cancels the subscription to the server-side signal provider. + */ + cancel(): void { + this.#manager.cancel(); + } +} diff --git a/packages/ts/react-signals/src/SignalChannel.ts b/packages/ts/react-signals/src/SignalChannel.ts deleted file mode 100644 index 1e1be28018..0000000000 --- a/packages/ts/react-signals/src/SignalChannel.ts +++ /dev/null @@ -1,100 +0,0 @@ -import type { ConnectClient, Subscription } from '@vaadin/hilla-frontend'; -import { nanoid } from 'nanoid'; -import { batch } from './core.js'; -import type { ValueSignal } from './Signals.js'; - -const ENDPOINT = 'SignalsHandler'; - -/** - * Types of changes that can be produced or processed by a signal. - */ -export enum StateEventType { - SET = 'set', - SNAPSHOT = 'snapshot', -} - -/** - * An object that describes the change of the signal state. - */ -export type StateEvent = Readonly<{ - id: string; - type: StateEventType; - value: unknown; -}>; - -/** - * A signal channel that can be used to communicate with a server-side. - * - * The signal channel is responsible for subscribing to the server-side signal - * and updating the local signal based on the received events. - * - * @typeParam S - The type of the signal instance. - */ -export class SignalChannel { - readonly #id = nanoid(); - readonly #client: ConnectClient; - readonly #method: string; - #dependencies: readonly ValueSignal[] = []; - #subscription?: Subscription; - - /** - * @param client - The client instance to be used for communication. - * @param signalProviderEndpointMethod - The method name of the signal provider - * service. - * @returns The signal channel instance. - */ - constructor(client: ConnectClient, signalProviderEndpointMethod: string) { - this.#client = client; - this.#method = signalProviderEndpointMethod; - } - - get id(): string { - return this.#id; - } - - cancel(): void { - this.#subscription?.cancel(); - this.#subscription = undefined; - } - - /** - * Connects the local signal to the server. - * - * @param signal - The signal instance to be connected. - * @param onUpdate - The callback that will be called when the signal is - * updated. - */ - connect(signal: ValueSignal, onUpdate: (promise: Promise) => void): void { - let paused = false; - - this.#dependencies = [...this.#dependencies, signal]; - - this.#subscription ??= this.#client - .subscribe(ENDPOINT, 'subscribe', { - signalProviderEndpointMethod: this.#method, - clientSignalId: this.#id, - }) - .onNext((event: StateEvent) => { - if (event.type === StateEventType.SNAPSHOT) { - paused = true; - batch(() => { - for (const dependency of this.#dependencies) { - dependency.value = event.value; - } - }); - paused = false; - } - }); - - signal.subscribe((value) => { - if (!paused) { - onUpdate( - this.#client.call(ENDPOINT, 'update', { - clientSignalId: this.#id, - event: { id: nanoid(), type: StateEventType.SET, value }, - }), - ); - } - }); - } -} diff --git a/packages/ts/react-signals/src/Signals.ts b/packages/ts/react-signals/src/Signals.ts index 767f8bfc65..c3436bf668 100644 --- a/packages/ts/react-signals/src/Signals.ts +++ b/packages/ts/react-signals/src/Signals.ts @@ -1,50 +1,9 @@ -import { signal } from '@preact/signals-react'; -import { Signal } from './core.js'; -import type { SignalChannel } from './SignalChannel.js'; - -export type ValueSignalOptions = Readonly<{ - channel?: SignalChannel; -}>; +import { FullStackSignal } from './FullStackSignal.js'; /** - * A signal that holds a value. The underlying - * value of this signal is stored and updated as a - * shared value on the server. - * - * @internal + * A full-stack signal that holds an arbitrary value. */ -export class ValueSignal extends Signal { - readonly #pending = signal(false); - readonly #error = signal(undefined); - - constructor(value?: T, options?: ValueSignalOptions) { - super(value); - options?.channel?.connect(this, (promise) => { - this.#pending.value = true; - promise - .catch((error: unknown) => { - this.#error.value = error instanceof Error ? error : new Error(String(error)); - }) - .finally(() => { - this.#pending.value = false; - }); - }); - } - - /** - * Defines whether the signal is currently pending server-side response. - */ - get pending(): boolean { - return this.#pending.value; - } - - /** - * Defines whether the signal has an error. - */ - get error(): Error | undefined { - return this.#error.value; - } -} +export class ValueSignal extends FullStackSignal {} /** * A signal that holds a number value. The underlying diff --git a/packages/ts/react-signals/src/index.ts b/packages/ts/react-signals/src/index.ts index 5a81d109af..f90c545d41 100644 --- a/packages/ts/react-signals/src/index.ts +++ b/packages/ts/react-signals/src/index.ts @@ -1,4 +1,4 @@ // eslint-disable-next-line import/export export * from './core.js'; -export { SignalChannel } from './SignalChannel.js'; -export { NumberSignal, ValueSignal } from './Signals.js'; +export { NumberSignal } from './Signals.js'; +export { FullStackSignal } from './FullStackSignal.js'; diff --git a/packages/ts/react-signals/test/SignalChannel.spec.tsx b/packages/ts/react-signals/test/FullStackSignal.spec.tsx similarity index 53% rename from packages/ts/react-signals/test/SignalChannel.spec.tsx rename to packages/ts/react-signals/test/FullStackSignal.spec.tsx index e79bc1e8ff..0a6b978bee 100644 --- a/packages/ts/react-signals/test/SignalChannel.spec.tsx +++ b/packages/ts/react-signals/test/FullStackSignal.spec.tsx @@ -4,33 +4,32 @@ import { render } from '@testing-library/react'; import { ConnectClient, type Subscription } from '@vaadin/hilla-frontend'; import sinon from 'sinon'; import sinonChai from 'sinon-chai'; -import { NumberSignal, SignalChannel } from '../src/index.js'; -import { StateEventType, type StateEvent } from '../src/SignalChannel.js'; +import { type StateEvent, StateEventType } from '../src/FullStackSignal.js'; +import { NumberSignal } from '../src/index.js'; import { nextFrame } from './utils.js'; use(sinonChai); describe('@vaadin/hilla-react-signals', () => { - describe('SignalChannel', () => { + describe('FullStackSignal', () => { function simulateReceivedChange( - connectSubscriptionMock: sinon.SinonSpiedInstance>, - event: StateEvent, + connectSubscriptionMock: sinon.SinonSpiedInstance>>, + event: StateEvent, ) { - const onNextCallback = connectSubscriptionMock.onNext.getCall(0).args[0]; + const [onNextCallback] = connectSubscriptionMock.onNext.firstCall.args; // eslint-disable-next-line @typescript-eslint/no-unsafe-call onNextCallback(event); } - let connectClientMock: sinon.SinonStubbedInstance; - let connectSubscriptionMock: sinon.SinonSpiedInstance>; + let client: sinon.SinonStubbedInstance; + let subscription: sinon.SinonSpiedInstance>>; let signal: NumberSignal; - let channel: SignalChannel; beforeEach(() => { - connectClientMock = sinon.createStubInstance(ConnectClient); - connectClientMock.call.resolves(); + client = sinon.createStubInstance(ConnectClient); + client.call.resolves(); - connectSubscriptionMock = sinon.spy>({ + subscription = sinon.spy>>({ cancel() {}, context() { return this; @@ -46,11 +45,10 @@ describe('@vaadin/hilla-react-signals', () => { }, }); // Mock the subscribe method - connectClientMock.subscribe.returns(connectSubscriptionMock); + client.subscribe.returns(subscription); - channel = new SignalChannel(connectClientMock, 'testEndpoint'); - signal = new NumberSignal(undefined, { channel }); - connectClientMock.call.resetHistory(); + signal = new NumberSignal(undefined, { client, method: 'testEndpoint' }); + client.call.resetHistory(); }); afterEach(() => { @@ -63,9 +61,9 @@ describe('@vaadin/hilla-react-signals', () => { }); it('should subscribe to signal provider endpoint', () => { - expect(connectClientMock.subscribe).to.be.have.been.calledOnce; - expect(connectClientMock.subscribe).to.have.been.calledWith('SignalsHandler', 'subscribe', { - clientSignalId: channel.id, + expect(client.subscribe).to.be.have.been.calledOnce; + expect(client.subscribe).to.have.been.calledWith('SignalsHandler', 'subscribe', { + clientSignalId: signal.id, signalProviderEndpointMethod: 'testEndpoint', }); }); @@ -73,9 +71,9 @@ describe('@vaadin/hilla-react-signals', () => { it('should publish updates to signals handler endpoint', () => { signal.value = 42; - expect(connectClientMock.call).to.be.have.been.calledOnce; - expect(connectClientMock.call).to.have.been.calledWithMatch('SignalsHandler', 'update', { - clientSignalId: channel.id, + expect(client.call).to.be.have.been.calledOnce; + expect(client.call).to.have.been.calledWithMatch('SignalsHandler', 'update', { + clientSignalId: signal.id, event: { type: StateEventType.SET, value: 42 }, }); }); @@ -84,8 +82,8 @@ describe('@vaadin/hilla-react-signals', () => { expect(signal.value).to.be.undefined; // Simulate the event received from the server: - const snapshotEvent: StateEvent = { id: 'someId', type: StateEventType.SNAPSHOT, value: 42 }; - simulateReceivedChange(connectSubscriptionMock, snapshotEvent); + const snapshotEvent: StateEvent = { id: 'someId', type: StateEventType.SNAPSHOT, value: 42 }; + simulateReceivedChange(subscription, snapshotEvent); // Check if the signal value is updated: expect(signal.value).to.equal(42); @@ -93,45 +91,45 @@ describe('@vaadin/hilla-react-signals', () => { it('should render the updated value', async () => { const numberSignal = signal; - simulateReceivedChange(connectSubscriptionMock, { id: 'someId', type: StateEventType.SNAPSHOT, value: 42 }); + simulateReceivedChange(subscription, { id: 'someId', type: StateEventType.SNAPSHOT, value: 42 }); const result = render(Value is {numberSignal}); await nextFrame(); expect(result.container.textContent).to.equal('Value is 42'); - simulateReceivedChange(connectSubscriptionMock, { id: 'someId', type: StateEventType.SNAPSHOT, value: 99 }); + simulateReceivedChange(subscription, { id: 'someId', type: StateEventType.SNAPSHOT, value: 99 }); await nextFrame(); expect(result.container.textContent).to.equal('Value is 99'); }); it('should subscribe using client', () => { - expect(connectClientMock.subscribe).to.be.have.been.calledOnce; - expect(connectClientMock.subscribe).to.have.been.calledWith('SignalsHandler', 'subscribe', { + expect(client.subscribe).to.be.have.been.calledOnce; + expect(client.subscribe).to.have.been.calledWith('SignalsHandler', 'subscribe', { signalProviderEndpointMethod: 'testEndpoint', - clientSignalId: channel.id, + clientSignalId: signal.id, }); }); it('should publish the new value to the server when set', () => { signal.value = 42; - expect(connectClientMock.call).to.have.been.calledOnce; - expect(connectClientMock.call).to.have.been.calledWithMatch('SignalsHandler', 'update', { + expect(client.call).to.have.been.calledOnce; + expect(client.call).to.have.been.calledWithMatch('SignalsHandler', 'update', { event: { type: StateEventType.SET, value: 42 }, }); signal.value = 0; - connectClientMock.call.resetHistory(); + client.call.resetHistory(); signal.value += 1; - expect(connectClientMock.call).to.have.been.calledOnce; - expect(connectClientMock.call).to.have.been.calledWithMatch('SignalsHandler', 'update', { + expect(client.call).to.have.been.calledOnce; + expect(client.call).to.have.been.calledWithMatch('SignalsHandler', 'update', { event: { type: StateEventType.SET, value: 1 }, }); }); it('should throw an error when the server call fails', () => { - connectClientMock.call.rejects(new Error('Server error')); + client.call.rejects(new Error('Server error')); signal.value = 42; }); }); diff --git a/packages/ts/react-signals/test/Signals.spec.tsx b/packages/ts/react-signals/test/Signals.spec.tsx index 55e1900405..720c242cde 100644 --- a/packages/ts/react-signals/test/Signals.spec.tsx +++ b/packages/ts/react-signals/test/Signals.spec.tsx @@ -1,48 +1,74 @@ /* eslint-disable @typescript-eslint/unbound-method */ import { expect, use } from '@esm-bundle/chai'; import { render } from '@testing-library/react'; +import { ConnectClient, type Subscription } from '@vaadin/hilla-frontend'; import chaiLike from 'chai-like'; +import sinon from 'sinon'; import sinonChai from 'sinon-chai'; import { effect } from '../src'; import { NumberSignal } from '../src'; +import type { FullStackSignalOptions, StateEvent } from '../src/FullStackSignal.js'; import { nextFrame } from './utils.js'; use(sinonChai); use(chaiLike); describe('@vaadin/hilla-react-signals', () => { + let options: FullStackSignalOptions; + + beforeEach(() => { + const client = sinon.createStubInstance(ConnectClient); + client.call.resolves(); + + const subscription = sinon.spy>>({ + cancel() {}, + context() { + return this; + }, + onComplete() { + return this; + }, + onError() { + return this; + }, + onNext() { + return this; + }, + }); + // Mock the subscribe method + client.subscribe.returns(subscription); + options = { client, method: 'testEndpoint' }; + }); + describe('NumberSignal', () => { it('should retain default value as initialized', () => { - const numberSignal1 = new NumberSignal(); + const numberSignal1 = new NumberSignal(undefined, options); expect(numberSignal1.value).to.be.undefined; - const numberSignal2 = new NumberSignal(undefined); - expect(numberSignal2.value).to.be.undefined; - - const numberSignal3 = new NumberSignal(0); - expect(numberSignal3.value).to.equal(0); + const numberSignal2 = new NumberSignal(0, options); + expect(numberSignal2.value).to.equal(0); - const numberSignal4 = new NumberSignal(42.424242); - expect(numberSignal4.value).to.equal(42.424242); + const numberSignal3 = new NumberSignal(42.424242, options); + expect(numberSignal3.value).to.equal(42.424242); - const numberSignal5 = new NumberSignal(-42.424242); - expect(numberSignal5.value).to.equal(-42.424242); + const numberSignal4 = new NumberSignal(-42.424242, options); + expect(numberSignal4.value).to.equal(-42.424242); }); it('should render value when signal is rendered', async () => { - const numberSignal = new NumberSignal(42); + const numberSignal = new NumberSignal(42, options); const result = render(Value is {numberSignal}); await nextFrame(); expect(result.container.textContent).to.equal('Value is 42'); }); it('should set the underlying value locally without waiting for server confirmation', () => { - const numberSignal = new NumberSignal(); + const numberSignal = new NumberSignal(undefined, options); expect(numberSignal.value).to.equal(undefined); numberSignal.value = 42; expect(numberSignal.value).to.equal(42); - const anotherNumberSignal = new NumberSignal(); + const anotherNumberSignal = new NumberSignal(undefined, options); const results: Array = []; effect(() => { From 6ba6167d6bd7dd9d94d8a895c198c0dd1a2026c1 Mon Sep 17 00:00:00 2001 From: Vlad Rindevich Date: Thu, 15 Aug 2024 17:16:14 +0300 Subject: [PATCH 13/22] refactor(generator-plugin-signals): update code --- .../generator-plugin-signals/src/SignalProcessor.ts | 13 ++----------- .../test/fixtures/NumberSignalServiceMix.snap.ts | 8 +++----- .../fixtures/NumberSignalServiceSignalOnly.snap.ts | 8 +++----- 3 files changed, 8 insertions(+), 21 deletions(-) diff --git a/packages/ts/generator-plugin-signals/src/SignalProcessor.ts b/packages/ts/generator-plugin-signals/src/SignalProcessor.ts index 913ad54a42..c56f276866 100644 --- a/packages/ts/generator-plugin-signals/src/SignalProcessor.ts +++ b/packages/ts/generator-plugin-signals/src/SignalProcessor.ts @@ -1,6 +1,5 @@ import type Plugin from '@vaadin/hilla-generator-core/Plugin.js'; import { template, transform, traverse } from '@vaadin/hilla-generator-utils/ast.js'; -import createFullyUniqueIdentifier from '@vaadin/hilla-generator-utils/createFullyUniqueIdentifier.js'; import createSourceFile from '@vaadin/hilla-generator-utils/createSourceFile.js'; import DependencyManager from '@vaadin/hilla-generator-utils/dependencies/DependencyManager.js'; import PathManager from '@vaadin/hilla-generator-utils/dependencies/PathManager.js'; @@ -8,8 +7,6 @@ import ts, { type FunctionDeclaration, type Identifier, type SourceFile } from ' const HILLA_REACT_SIGNALS = '@vaadin/hilla-react-signals'; -const CHANNEL = '$CHANNEL$'; -const SIGNAL_CHANNEL = '$SIGNAL_CHANNEL$'; const CONNECT_CLIENT = '$CONNECT_CLIENT$'; const METHOD_NAME = '$METHOD_NAME$'; const SIGNAL = '$SIGNAL$'; @@ -36,8 +33,6 @@ export default class SignalProcessor { this.#owner.logger.debug(`Processing signals: ${this.#service}`); const { imports } = this.#dependencyManager; - const signalChannelId = imports.named.add(HILLA_REACT_SIGNALS, 'SignalChannel'); - const [, connectClientId] = imports.default.iter().find(([path]) => path.includes('connect-client'))!; const initTypeId = imports.named.getIdentifier('@vaadin/hilla-frontend', 'EndpointRequestInit'); @@ -47,18 +42,14 @@ export default class SignalProcessor { transform((tsNode) => { if (ts.isFunctionDeclaration(tsNode) && tsNode.name && this.#methods.has(tsNode.name.text)) { const signalId = this.#replaceSignalImport(tsNode); - const channel = createFullyUniqueIdentifier('channel'); return template( - `const ${CHANNEL} = new ${SIGNAL_CHANNEL}(${CONNECT_CLIENT}, '${this.#service}.${tsNode.name.text}') -function ${METHOD_NAME}() { - return new ${SIGNAL}(undefined, { channel: ${CHANNEL} }); + `function ${METHOD_NAME}() { + return new ${SIGNAL}(undefined, { client: ${CONNECT_CLIENT}, method: '${this.#service}.${tsNode.name.text}' }); }`, (statements) => statements, [ - transform((node) => (ts.isIdentifier(node) && node.text === CHANNEL ? channel : node)), transform((node) => (ts.isIdentifier(node) && node.text === METHOD_NAME ? tsNode.name : node)), - transform((node) => (ts.isIdentifier(node) && node.text === SIGNAL_CHANNEL ? signalChannelId : node)), transform((node) => (ts.isIdentifier(node) && node.text === SIGNAL ? signalId : node)), transform((node) => (ts.isIdentifier(node) && node.text === CONNECT_CLIENT ? connectClientId : node)), ], diff --git a/packages/ts/generator-plugin-signals/test/fixtures/NumberSignalServiceMix.snap.ts b/packages/ts/generator-plugin-signals/test/fixtures/NumberSignalServiceMix.snap.ts index 65d0338ea2..541fcdfb5f 100644 --- a/packages/ts/generator-plugin-signals/test/fixtures/NumberSignalServiceMix.snap.ts +++ b/packages/ts/generator-plugin-signals/test/fixtures/NumberSignalServiceMix.snap.ts @@ -1,13 +1,11 @@ import { EndpointRequestInit as EndpointRequestInit_1 } from "@vaadin/hilla-frontend"; -import { NumberSignal as NumberSignal_1, SignalChannel as SignalChannel_1 } from "@vaadin/hilla-react-signals"; +import { NumberSignal as NumberSignal_1 } from "@vaadin/hilla-react-signals"; import client_1 from "./connect-client.default.js"; -const channel_1 = new SignalChannel_1(client_1, "NumberSignalService.counter"); function counter_1() { - return new NumberSignal_1(undefined, { channel: channel_1 }); + return new NumberSignal_1(undefined, { client: client_1, method: "NumberSignalService.counter" }); } async function sayHello_1(name: string, init?: EndpointRequestInit_1): Promise { return client_1.call("NumberSignalService", "sayHello", { name }, init); } -const channel_2 = new SignalChannel_1(client_1, "NumberSignalService.sharedValue"); function sharedValue_1() { - return new NumberSignal_1(undefined, { channel: channel_2 }); + return new NumberSignal_1(undefined, { client: client_1, method: "NumberSignalService.sharedValue" }); } export { counter_1 as counter, sayHello_1 as sayHello, sharedValue_1 as sharedValue }; diff --git a/packages/ts/generator-plugin-signals/test/fixtures/NumberSignalServiceSignalOnly.snap.ts b/packages/ts/generator-plugin-signals/test/fixtures/NumberSignalServiceSignalOnly.snap.ts index 48a11b0ed8..a6061f41eb 100644 --- a/packages/ts/generator-plugin-signals/test/fixtures/NumberSignalServiceSignalOnly.snap.ts +++ b/packages/ts/generator-plugin-signals/test/fixtures/NumberSignalServiceSignalOnly.snap.ts @@ -1,11 +1,9 @@ -import { NumberSignal as NumberSignal_1, SignalChannel as SignalChannel_1 } from "@vaadin/hilla-react-signals"; +import { NumberSignal as NumberSignal_1 } from "@vaadin/hilla-react-signals"; import client_1 from "./connect-client.default.js"; -const channel_1 = new SignalChannel_1(client_1, "NumberSignalService.counter"); function counter_1() { - return new NumberSignal_1(undefined, { channel: channel_1 }); + return new NumberSignal_1(undefined, { client: client_1, method: "NumberSignalService.counter" }); } -const channel_2 = new SignalChannel_1(client_1, "NumberSignalService.sharedValue"); function sharedValue_1() { - return new NumberSignal_1(undefined, { channel: channel_2 }); + return new NumberSignal_1(undefined, { client: client_1, method: "NumberSignalService.sharedValue" }); } export { counter_1 as counter, sharedValue_1 as sharedValue }; From f5b2adcfa41450422f78c758b2dac4cde81796cc Mon Sep 17 00:00:00 2001 From: Vlad Rindevich Date: Thu, 15 Aug 2024 17:27:13 +0300 Subject: [PATCH 14/22] refactor: split endpoint and method names --- .../hilla/signals/handler/SignalsHandler.java | 15 ++++---- .../signals/handler/SignalsHandlerTest.java | 12 +++---- .../src/SignalProcessor.ts | 2 +- .../fixtures/NumberSignalServiceMix.snap.ts | 4 +-- .../ts/react-signals/src/FullStackSignal.ts | 34 ++++++++++++------- .../test/FullStackSignal.spec.tsx | 8 +++-- .../ts/react-signals/test/Signals.spec.tsx | 4 +-- 7 files changed, 43 insertions(+), 36 deletions(-) diff --git a/packages/java/endpoint/src/main/java/com/vaadin/hilla/signals/handler/SignalsHandler.java b/packages/java/endpoint/src/main/java/com/vaadin/hilla/signals/handler/SignalsHandler.java index 71d0896ea1..4e241aa13e 100644 --- a/packages/java/endpoint/src/main/java/com/vaadin/hilla/signals/handler/SignalsHandler.java +++ b/packages/java/endpoint/src/main/java/com/vaadin/hilla/signals/handler/SignalsHandler.java @@ -23,27 +23,24 @@ public SignalsHandler(SecureSignalsRegistry registry) { /** * Subscribes to a signal. * - * @param signalProviderEndpointMethod + * @param providerEndpoint + * the endpoint that provides the signal + * @param providerMethod * the endpoint method that provides the signal * @param clientSignalId * the client signal id * * @return a Flux of JSON events */ - public Flux subscribe(String signalProviderEndpointMethod, - String clientSignalId) { + public Flux subscribe(String providerEndpoint, + String providerMethod, String clientSignalId) { try { - String[] endpointMethodParts = signalProviderEndpointMethod - .split("\\."); - var endpointName = endpointMethodParts[0]; - var methodName = endpointMethodParts[1]; - var signal = registry.get(clientSignalId); if (signal != null) { return signal.subscribe(); } - registry.register(clientSignalId, endpointName, methodName); + registry.register(clientSignalId, providerEndpoint, providerMethod); return registry.get(clientSignalId).subscribe(); } catch (Exception e) { return Flux.error(e); diff --git a/packages/java/endpoint/src/test/java/com/vaadin/hilla/signals/handler/SignalsHandlerTest.java b/packages/java/endpoint/src/test/java/com/vaadin/hilla/signals/handler/SignalsHandlerTest.java index d933c894bc..42b1a69ff5 100644 --- a/packages/java/endpoint/src/test/java/com/vaadin/hilla/signals/handler/SignalsHandlerTest.java +++ b/packages/java/endpoint/src/test/java/com/vaadin/hilla/signals/handler/SignalsHandlerTest.java @@ -49,8 +49,8 @@ public void when_signalAlreadyRegistered_subscribe_returnsSubscriptionOfSameInst .put("type", "snapshot"); // first client subscribe to a signal, it registers the signal: - Flux firstFlux = signalsHandler.subscribe("endpoint.method", - CLIENT_SIGNAL_ID_1); + Flux firstFlux = signalsHandler.subscribe("endpoint", + "method", CLIENT_SIGNAL_ID_1); firstFlux.subscribe(next -> { assertNotNull(next); assertEquals(expectedSignalEventJson, next); @@ -59,8 +59,8 @@ public void when_signalAlreadyRegistered_subscribe_returnsSubscriptionOfSameInst }); // another client subscribes to the same signal: - Flux secondFlux = signalsHandler - .subscribe("endpoint.method", CLIENT_SIGNAL_ID_2); + Flux secondFlux = signalsHandler.subscribe("endpoint", + "method", CLIENT_SIGNAL_ID_2); secondFlux.subscribe(next -> { assertNotNull(next); assertEquals(expectedSignalEventJson, next); @@ -84,8 +84,8 @@ public void when_signalIsRegistered_update_notifiesTheSubscribers() var signalId = numberSignal.getId(); when(signalsRegistry.get(CLIENT_SIGNAL_ID_1)).thenReturn(numberSignal); - Flux firstFlux = signalsHandler.subscribe("endpoint.method", - CLIENT_SIGNAL_ID_1); + Flux firstFlux = signalsHandler.subscribe("endpoint", + "method", CLIENT_SIGNAL_ID_1); var setEvent = new ObjectNode(mapper.getNodeFactory()).put("value", 42) .put("id", UUID.randomUUID().toString()).put("type", "set"); diff --git a/packages/ts/generator-plugin-signals/src/SignalProcessor.ts b/packages/ts/generator-plugin-signals/src/SignalProcessor.ts index c56f276866..c29f704da2 100644 --- a/packages/ts/generator-plugin-signals/src/SignalProcessor.ts +++ b/packages/ts/generator-plugin-signals/src/SignalProcessor.ts @@ -45,7 +45,7 @@ export default class SignalProcessor { return template( `function ${METHOD_NAME}() { - return new ${SIGNAL}(undefined, { client: ${CONNECT_CLIENT}, method: '${this.#service}.${tsNode.name.text}' }); + return new ${SIGNAL}(undefined, { client: ${CONNECT_CLIENT}, endpoint: '${this.#service}', method: '${tsNode.name.text}' }); }`, (statements) => statements, [ diff --git a/packages/ts/generator-plugin-signals/test/fixtures/NumberSignalServiceMix.snap.ts b/packages/ts/generator-plugin-signals/test/fixtures/NumberSignalServiceMix.snap.ts index 541fcdfb5f..877a588e36 100644 --- a/packages/ts/generator-plugin-signals/test/fixtures/NumberSignalServiceMix.snap.ts +++ b/packages/ts/generator-plugin-signals/test/fixtures/NumberSignalServiceMix.snap.ts @@ -2,10 +2,10 @@ import { EndpointRequestInit as EndpointRequestInit_1 } from "@vaadin/hilla-fron import { NumberSignal as NumberSignal_1 } from "@vaadin/hilla-react-signals"; import client_1 from "./connect-client.default.js"; function counter_1() { - return new NumberSignal_1(undefined, { client: client_1, method: "NumberSignalService.counter" }); + return new NumberSignal_1(undefined, { client: client_1, endpoint: "NumberSignalService", method: "counter" }); } async function sayHello_1(name: string, init?: EndpointRequestInit_1): Promise { return client_1.call("NumberSignalService", "sayHello", { name }, init); } function sharedValue_1() { - return new NumberSignal_1(undefined, { client: client_1, method: "NumberSignalService.sharedValue" }); + return new NumberSignal_1(undefined, { client: client_1, endpoint: "NumberSignalService", method: "sharedValue" }); } export { counter_1 as counter, sayHello_1 as sayHello, sharedValue_1 as sharedValue }; diff --git a/packages/ts/react-signals/src/FullStackSignal.ts b/packages/ts/react-signals/src/FullStackSignal.ts index bc9bc637f2..a0fdad7bad 100644 --- a/packages/ts/react-signals/src/FullStackSignal.ts +++ b/packages/ts/react-signals/src/FullStackSignal.ts @@ -27,28 +27,33 @@ export type StateEvent = Readonly<{ value: T; }>; /** - * Options for creating a full-stack signal. + * An object that describes a data object to connect to the signal provider + * service. */ -export type FullStackSignalOptions = Readonly<{ +export type ConnectionData = Readonly<{ /** * The client instance to be used for communication. */ client: ConnectClient; + /** - * The method name of the signal provider service. + * The name of the signal provider service endpoint. + */ + endpoint: string; + + /** + * The name of the signal provider service method. */ method: string; }>; class SubscriptionManager { - readonly #client; - readonly #method: string; readonly #id: string; + readonly #data: ConnectionData; #subscription?: Subscription>; - constructor(client: ConnectClient, method: string, id: string) { - this.#client = client; - this.#method = method; + constructor(id: string, data: ConnectionData) { + this.#data = data; this.#id = id; } @@ -57,8 +62,11 @@ class SubscriptionManager { } subscribe() { - this.#subscription ??= this.#client.subscribe(ENDPOINT, 'subscribe', { - signalProviderEndpointMethod: this.#method, + const { client, endpoint, method } = this.#data; + + this.#subscription ??= client.subscribe(ENDPOINT, 'subscribe', { + providerEndpoint: endpoint, + providerMethod: method, clientSignalId: this.#id, }); @@ -66,7 +74,7 @@ class SubscriptionManager { } async call(event: SubscriptionEvent): Promise { - await this.#client.call(ENDPOINT, 'update', { + await this.#data.client.call(ENDPOINT, 'update', { clientSignalId: this.#id, event, }); @@ -92,9 +100,9 @@ export abstract class FullStackSignal extends Signal { readonly #error = signal(undefined); readonly #manager: SubscriptionManager; - constructor(value: T | undefined, { client, method }: FullStackSignalOptions) { + constructor(value: T | undefined, data: ConnectionData) { super(value); - this.#manager = new SubscriptionManager(client, method, this.id); + this.#manager = new SubscriptionManager(this.id, data); let paused = false; this.#manager.subscribe().onNext((event: StateEvent) => { diff --git a/packages/ts/react-signals/test/FullStackSignal.spec.tsx b/packages/ts/react-signals/test/FullStackSignal.spec.tsx index 0a6b978bee..16ceeddd3c 100644 --- a/packages/ts/react-signals/test/FullStackSignal.spec.tsx +++ b/packages/ts/react-signals/test/FullStackSignal.spec.tsx @@ -47,7 +47,7 @@ describe('@vaadin/hilla-react-signals', () => { // Mock the subscribe method client.subscribe.returns(subscription); - signal = new NumberSignal(undefined, { client, method: 'testEndpoint' }); + signal = new NumberSignal(undefined, { client, endpoint: 'TestEndpoint', method: 'testMethod' }); client.call.resetHistory(); }); @@ -64,7 +64,8 @@ describe('@vaadin/hilla-react-signals', () => { expect(client.subscribe).to.be.have.been.calledOnce; expect(client.subscribe).to.have.been.calledWith('SignalsHandler', 'subscribe', { clientSignalId: signal.id, - signalProviderEndpointMethod: 'testEndpoint', + providerEndpoint: 'TestEndpoint', + providerMethod: 'testMethod', }); }); @@ -105,8 +106,9 @@ describe('@vaadin/hilla-react-signals', () => { it('should subscribe using client', () => { expect(client.subscribe).to.be.have.been.calledOnce; expect(client.subscribe).to.have.been.calledWith('SignalsHandler', 'subscribe', { - signalProviderEndpointMethod: 'testEndpoint', clientSignalId: signal.id, + providerEndpoint: 'TestEndpoint', + providerMethod: 'testMethod', }); }); diff --git a/packages/ts/react-signals/test/Signals.spec.tsx b/packages/ts/react-signals/test/Signals.spec.tsx index 720c242cde..b362e99630 100644 --- a/packages/ts/react-signals/test/Signals.spec.tsx +++ b/packages/ts/react-signals/test/Signals.spec.tsx @@ -7,14 +7,14 @@ import sinon from 'sinon'; import sinonChai from 'sinon-chai'; import { effect } from '../src'; import { NumberSignal } from '../src'; -import type { FullStackSignalOptions, StateEvent } from '../src/FullStackSignal.js'; +import type { ConnectionData, StateEvent } from '../src/FullStackSignal.js'; import { nextFrame } from './utils.js'; use(sinonChai); use(chaiLike); describe('@vaadin/hilla-react-signals', () => { - let options: FullStackSignalOptions; + let options: ConnectionData; beforeEach(() => { const client = sinon.createStubInstance(ConnectClient); From ab91b7eddbd1294256ade2ecd9da936d7da94fbe Mon Sep 17 00:00:00 2001 From: Vlad Rindevich Date: Thu, 15 Aug 2024 17:29:21 +0300 Subject: [PATCH 15/22] refactor(react-signals): remove unnecessary name --- packages/ts/generator-plugin-signals/src/SignalProcessor.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/ts/generator-plugin-signals/src/SignalProcessor.ts b/packages/ts/generator-plugin-signals/src/SignalProcessor.ts index c29f704da2..f48a356ce6 100644 --- a/packages/ts/generator-plugin-signals/src/SignalProcessor.ts +++ b/packages/ts/generator-plugin-signals/src/SignalProcessor.ts @@ -11,7 +11,7 @@ const CONNECT_CLIENT = '$CONNECT_CLIENT$'; const METHOD_NAME = '$METHOD_NAME$'; const SIGNAL = '$SIGNAL$'; -const signals = ['NumberSignal', 'RemoteSignal']; +const signals = ['NumberSignal']; export default class SignalProcessor { readonly #dependencyManager: DependencyManager; From 4dfea71b1215d7e21571389111228ccdd7da815c Mon Sep 17 00:00:00 2001 From: Vlad Rindevich Date: Fri, 16 Aug 2024 12:09:53 +0300 Subject: [PATCH 16/22] test(react-signals): update code --- packages/ts/react-signals/test/Signals.spec.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/ts/react-signals/test/Signals.spec.tsx b/packages/ts/react-signals/test/Signals.spec.tsx index b362e99630..84fe6a8509 100644 --- a/packages/ts/react-signals/test/Signals.spec.tsx +++ b/packages/ts/react-signals/test/Signals.spec.tsx @@ -37,7 +37,7 @@ describe('@vaadin/hilla-react-signals', () => { }); // Mock the subscribe method client.subscribe.returns(subscription); - options = { client, method: 'testEndpoint' }; + options = { client, endpoint: 'TestEndpoint', method: 'testMethod' }; }); describe('NumberSignal', () => { From 1e603b780533ebdb17166518e23531d31b3ab07f Mon Sep 17 00:00:00 2001 From: Vlad Rindevich Date: Fri, 16 Aug 2024 12:10:59 +0300 Subject: [PATCH 17/22] test(generator-plugin-signals): update fixture --- .../test/fixtures/NumberSignalServiceSignalOnly.snap.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/ts/generator-plugin-signals/test/fixtures/NumberSignalServiceSignalOnly.snap.ts b/packages/ts/generator-plugin-signals/test/fixtures/NumberSignalServiceSignalOnly.snap.ts index a6061f41eb..01681ee518 100644 --- a/packages/ts/generator-plugin-signals/test/fixtures/NumberSignalServiceSignalOnly.snap.ts +++ b/packages/ts/generator-plugin-signals/test/fixtures/NumberSignalServiceSignalOnly.snap.ts @@ -1,9 +1,9 @@ import { NumberSignal as NumberSignal_1 } from "@vaadin/hilla-react-signals"; import client_1 from "./connect-client.default.js"; function counter_1() { - return new NumberSignal_1(undefined, { client: client_1, method: "NumberSignalService.counter" }); + return new NumberSignal_1(undefined, { client: client_1, endpoint: "NumberSignalService", method: "counter" }); } function sharedValue_1() { - return new NumberSignal_1(undefined, { client: client_1, method: "NumberSignalService.sharedValue" }); + return new NumberSignal_1(undefined, { client: client_1, endpoint: "NumberSignalService", method: "sharedValue" }); } export { counter_1 as counter, sharedValue_1 as sharedValue }; From 85708aaaecc19eb8d3ef0ce506d0bfc5a66e1a96 Mon Sep 17 00:00:00 2001 From: Vlad Rindevich Date: Fri, 16 Aug 2024 16:01:56 +0300 Subject: [PATCH 18/22] fix(react-signals): get "id" field back to a change event --- packages/ts/react-signals/src/FullStackSignal.ts | 2 ++ packages/ts/react-signals/test/FullStackSignal.spec.tsx | 4 ++++ 2 files changed, 6 insertions(+) diff --git a/packages/ts/react-signals/src/FullStackSignal.ts b/packages/ts/react-signals/src/FullStackSignal.ts index a0fdad7bad..7bd743f121 100644 --- a/packages/ts/react-signals/src/FullStackSignal.ts +++ b/packages/ts/react-signals/src/FullStackSignal.ts @@ -6,6 +6,7 @@ import { Signal } from './core.js'; const ENDPOINT = 'SignalsHandler'; type SubscriptionEvent = Readonly<{ + id: string; type: StateEventType; value: T; }>; @@ -118,6 +119,7 @@ export abstract class FullStackSignal extends Signal { this.#pending.value = true; this.#manager .call({ + id: nanoid(), type: StateEventType.SET, value: v, }) diff --git a/packages/ts/react-signals/test/FullStackSignal.spec.tsx b/packages/ts/react-signals/test/FullStackSignal.spec.tsx index 16ceeddd3c..61a956d6e7 100644 --- a/packages/ts/react-signals/test/FullStackSignal.spec.tsx +++ b/packages/ts/react-signals/test/FullStackSignal.spec.tsx @@ -128,6 +128,10 @@ describe('@vaadin/hilla-react-signals', () => { expect(client.call).to.have.been.calledWithMatch('SignalsHandler', 'update', { event: { type: StateEventType.SET, value: 1 }, }); + + const [, , params] = client.call.firstCall.args; + + expect(params!.event).to.have.property('id'); }); it('should throw an error when the server call fails', () => { From 353dc71692e8b3d2953290cda22d43277cffae9b Mon Sep 17 00:00:00 2001 From: Vlad Rindevich Date: Mon, 19 Aug 2024 10:52:15 +0300 Subject: [PATCH 19/22] refactor(react-signals): address review comments --- .../ts/react-signals/src/FullStackSignal.ts | 83 +++++++++++-------- .../test/FullStackSignal.spec.tsx | 9 ++ .../ts/react-signals/test/Signals.spec.tsx | 4 +- 3 files changed, 58 insertions(+), 38 deletions(-) diff --git a/packages/ts/react-signals/src/FullStackSignal.ts b/packages/ts/react-signals/src/FullStackSignal.ts index 7bd743f121..34ac48a99c 100644 --- a/packages/ts/react-signals/src/FullStackSignal.ts +++ b/packages/ts/react-signals/src/FullStackSignal.ts @@ -5,12 +5,6 @@ import { Signal } from './core.js'; const ENDPOINT = 'SignalsHandler'; -type SubscriptionEvent = Readonly<{ - id: string; - type: StateEventType; - value: T; -}>; - /** * Types of changes that can be produced or processed by a signal. */ @@ -27,11 +21,12 @@ export type StateEvent = Readonly<{ type: StateEventType; value: T; }>; + /** * An object that describes a data object to connect to the signal provider * service. */ -export type ConnectionData = Readonly<{ +export type ServerConnectionConfig = Readonly<{ /** * The client instance to be used for communication. */ @@ -48,13 +43,16 @@ export type ConnectionData = Readonly<{ method: string; }>; -class SubscriptionManager { +/** + * A server connection manager. + */ +class ServerConnection { readonly #id: string; - readonly #data: ConnectionData; + readonly #config: ServerConnectionConfig; #subscription?: Subscription>; - constructor(id: string, data: ConnectionData) { - this.#data = data; + constructor(id: string, config: ServerConnectionConfig) { + this.#config = config; this.#id = id; } @@ -62,8 +60,8 @@ class SubscriptionManager { return this.#subscription; } - subscribe() { - const { client, endpoint, method } = this.#data; + connect() { + const { client, endpoint, method } = this.#config; this.#subscription ??= client.subscribe(ENDPOINT, 'subscribe', { providerEndpoint: endpoint, @@ -74,14 +72,14 @@ class SubscriptionManager { return this.#subscription; } - async call(event: SubscriptionEvent): Promise { - await this.#data.client.call(ENDPOINT, 'update', { + async update(event: StateEvent): Promise { + await this.#config.client.call(ENDPOINT, 'update', { clientSignalId: this.#id, event, }); } - cancel() { + disconnect() { this.#subscription?.cancel(); this.#subscription = undefined; } @@ -96,17 +94,28 @@ class SubscriptionManager { * @internal */ export abstract class FullStackSignal extends Signal { + /** + * The unique identifier of the signal necessary to communicate with the + * server. + */ readonly id = nanoid(); + + /** + * The server connection manager. + */ + readonly server: ServerConnection; readonly #pending = signal(false); readonly #error = signal(undefined); - readonly #manager: SubscriptionManager; - constructor(value: T | undefined, data: ConnectionData) { + constructor(value: T | undefined, config: ServerConnectionConfig) { super(value); - this.#manager = new SubscriptionManager(this.id, data); + this.server = new ServerConnection(this.id, config); + + // Paused at the very start to prevent the signal from sending the initial + // value to the server. + let paused = true; - let paused = false; - this.#manager.subscribe().onNext((event: StateEvent) => { + this.server.connect().onNext((event: StateEvent) => { if (event.type === StateEventType.SNAPSHOT) { paused = true; this.value = event.value; @@ -117,8 +126,8 @@ export abstract class FullStackSignal extends Signal { this.subscribe((v) => { if (!paused) { this.#pending.value = true; - this.#manager - .call({ + this.server + .update({ id: nanoid(), type: StateEventType.SET, value: v, @@ -131,6 +140,7 @@ export abstract class FullStackSignal extends Signal { }); } }); + paused = false; } /** @@ -147,21 +157,22 @@ export abstract class FullStackSignal extends Signal { return this.#error.value; } + /** + * Waits the signal to receive the update from the server-side signal + * provider. + */ get updating(): Promise { return new Promise((resolve) => { - const unsubscribe = this.#pending.subscribe((value) => { - if (value) { - resolve(); - unsubscribe(); - } - }); + if (this.#pending.value) { + const unsubscribe = this.#pending.subscribe((value) => { + if (value) { + resolve(); + unsubscribe(); + } + }); + } else { + resolve(); + } }); } - - /** - * Cancels the subscription to the server-side signal provider. - */ - cancel(): void { - this.#manager.cancel(); - } } diff --git a/packages/ts/react-signals/test/FullStackSignal.spec.tsx b/packages/ts/react-signals/test/FullStackSignal.spec.tsx index 61a956d6e7..63de252c5b 100644 --- a/packages/ts/react-signals/test/FullStackSignal.spec.tsx +++ b/packages/ts/react-signals/test/FullStackSignal.spec.tsx @@ -134,6 +134,15 @@ describe('@vaadin/hilla-react-signals', () => { expect(params!.event).to.have.property('id'); }); + it('should provide an internal server subscription', () => { + expect(signal.server.subscription).to.equal(subscription); + }); + + it('should disconnect from the server', () => { + signal.server.disconnect(); + expect(subscription.cancel).to.have.been.calledOnce; + }); + it('should throw an error when the server call fails', () => { client.call.rejects(new Error('Server error')); signal.value = 42; diff --git a/packages/ts/react-signals/test/Signals.spec.tsx b/packages/ts/react-signals/test/Signals.spec.tsx index 84fe6a8509..650081f095 100644 --- a/packages/ts/react-signals/test/Signals.spec.tsx +++ b/packages/ts/react-signals/test/Signals.spec.tsx @@ -7,14 +7,14 @@ import sinon from 'sinon'; import sinonChai from 'sinon-chai'; import { effect } from '../src'; import { NumberSignal } from '../src'; -import type { ConnectionData, StateEvent } from '../src/FullStackSignal.js'; +import type { ServerConnectionConfig, StateEvent } from '../src/FullStackSignal.js'; import { nextFrame } from './utils.js'; use(sinonChai); use(chaiLike); describe('@vaadin/hilla-react-signals', () => { - let options: ConnectionData; + let options: ServerConnectionConfig; beforeEach(() => { const client = sinon.createStubInstance(ConnectClient); From 8f36c61a36dd53448b292025cfcf272452ed3b59 Mon Sep 17 00:00:00 2001 From: Vlad Rindevich Date: Mon, 19 Aug 2024 11:37:38 +0300 Subject: [PATCH 20/22] refactor(react-signals): return ReadonlySignal instead of a signal value --- .../ts/react-signals/src/FullStackSignal.ts | 51 ++++++------------- .../test/FullStackSignal.spec.tsx | 19 +++++++ 2 files changed, 34 insertions(+), 36 deletions(-) diff --git a/packages/ts/react-signals/src/FullStackSignal.ts b/packages/ts/react-signals/src/FullStackSignal.ts index 34ac48a99c..b19386bd81 100644 --- a/packages/ts/react-signals/src/FullStackSignal.ts +++ b/packages/ts/react-signals/src/FullStackSignal.ts @@ -1,4 +1,4 @@ -import { signal } from '@preact/signals-react'; +import { computed, signal } from '@preact/signals-react'; import type { ConnectClient, Subscription } from '@vaadin/hilla-frontend'; import { nanoid } from 'nanoid'; import { Signal } from './core.js'; @@ -104,6 +104,17 @@ export abstract class FullStackSignal extends Signal { * The server connection manager. */ readonly server: ServerConnection; + + /** + * Defines whether the signal is currently awaits a server-side response. + */ + readonly pending = computed(() => this.#pending.value); + + /** + * Defines whether the signal has an error. + */ + readonly error = computed(() => this.#error.value); + readonly #pending = signal(false); readonly #error = signal(undefined); @@ -132,47 +143,15 @@ export abstract class FullStackSignal extends Signal { type: StateEventType.SET, value: v, }) - .catch((error) => { - this.#error.value = error; + .catch((error: unknown) => { + this.#error.value = error instanceof Error ? error : new Error(String(error)); }) .finally(() => { this.#pending.value = false; }); } }); - paused = false; - } - - /** - * Defines whether the signal is currently awaits a server-side response. - */ - get pending(): boolean { - return this.#pending.value; - } - /** - * Defines whether the signal has an error. - */ - get error(): Error | undefined { - return this.#error.value; - } - - /** - * Waits the signal to receive the update from the server-side signal - * provider. - */ - get updating(): Promise { - return new Promise((resolve) => { - if (this.#pending.value) { - const unsubscribe = this.#pending.subscribe((value) => { - if (value) { - resolve(); - unsubscribe(); - } - }); - } else { - resolve(); - } - }); + paused = false; } } diff --git a/packages/ts/react-signals/test/FullStackSignal.spec.tsx b/packages/ts/react-signals/test/FullStackSignal.spec.tsx index 63de252c5b..e9b32906fd 100644 --- a/packages/ts/react-signals/test/FullStackSignal.spec.tsx +++ b/packages/ts/react-signals/test/FullStackSignal.spec.tsx @@ -134,6 +134,25 @@ describe('@vaadin/hilla-react-signals', () => { expect(params!.event).to.have.property('id'); }); + it('should provide a way to access connection errors', async () => { + const error = new Error('Server error'); + client.call.rejects(error); + + signal.value = 42; + // Waiting for the ConnectionClient#call promise to resolve. + await nextFrame(); + + expect(signal.error).to.have.property('value', error); + }); + + it('should provide a way to access the pending state', async () => { + expect(signal.pending).to.have.property('value', false); + signal.value = 42; + expect(signal.pending).to.have.property('value', true); + await nextFrame(); + expect(signal.pending).to.have.property('value', false); + }); + it('should provide an internal server subscription', () => { expect(signal.server.subscription).to.equal(subscription); }); From 8d2c52fbba05d79a8a0f47477260a4edb644d12e Mon Sep 17 00:00:00 2001 From: Vlad Rindevich Date: Mon, 19 Aug 2024 11:43:53 +0300 Subject: [PATCH 21/22] test(react-signals): use correct naming --- .../ts/react-signals/test/Signals.spec.tsx | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/packages/ts/react-signals/test/Signals.spec.tsx b/packages/ts/react-signals/test/Signals.spec.tsx index 650081f095..eaf60b3b23 100644 --- a/packages/ts/react-signals/test/Signals.spec.tsx +++ b/packages/ts/react-signals/test/Signals.spec.tsx @@ -14,7 +14,7 @@ use(sinonChai); use(chaiLike); describe('@vaadin/hilla-react-signals', () => { - let options: ServerConnectionConfig; + let config: ServerConnectionConfig; beforeEach(() => { const client = sinon.createStubInstance(ConnectClient); @@ -37,38 +37,38 @@ describe('@vaadin/hilla-react-signals', () => { }); // Mock the subscribe method client.subscribe.returns(subscription); - options = { client, endpoint: 'TestEndpoint', method: 'testMethod' }; + config = { client, endpoint: 'TestEndpoint', method: 'testMethod' }; }); describe('NumberSignal', () => { it('should retain default value as initialized', () => { - const numberSignal1 = new NumberSignal(undefined, options); + const numberSignal1 = new NumberSignal(undefined, config); expect(numberSignal1.value).to.be.undefined; - const numberSignal2 = new NumberSignal(0, options); + const numberSignal2 = new NumberSignal(0, config); expect(numberSignal2.value).to.equal(0); - const numberSignal3 = new NumberSignal(42.424242, options); + const numberSignal3 = new NumberSignal(42.424242, config); expect(numberSignal3.value).to.equal(42.424242); - const numberSignal4 = new NumberSignal(-42.424242, options); + const numberSignal4 = new NumberSignal(-42.424242, config); expect(numberSignal4.value).to.equal(-42.424242); }); it('should render value when signal is rendered', async () => { - const numberSignal = new NumberSignal(42, options); + const numberSignal = new NumberSignal(42, config); const result = render(Value is {numberSignal}); await nextFrame(); expect(result.container.textContent).to.equal('Value is 42'); }); it('should set the underlying value locally without waiting for server confirmation', () => { - const numberSignal = new NumberSignal(undefined, options); + const numberSignal = new NumberSignal(undefined, config); expect(numberSignal.value).to.equal(undefined); numberSignal.value = 42; expect(numberSignal.value).to.equal(42); - const anotherNumberSignal = new NumberSignal(undefined, options); + const anotherNumberSignal = new NumberSignal(undefined, config); const results: Array = []; effect(() => { From 47b0346544f6bc21ab2191da37267f0a080f364a Mon Sep 17 00:00:00 2001 From: Vlad Rindevich Date: Mon, 19 Aug 2024 11:48:54 +0300 Subject: [PATCH 22/22] fix(react-signals): reset error signal on the new call --- packages/ts/react-signals/src/FullStackSignal.ts | 1 + .../ts/react-signals/test/FullStackSignal.spec.tsx | 14 ++++++++++---- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/packages/ts/react-signals/src/FullStackSignal.ts b/packages/ts/react-signals/src/FullStackSignal.ts index b19386bd81..ea68f45b9e 100644 --- a/packages/ts/react-signals/src/FullStackSignal.ts +++ b/packages/ts/react-signals/src/FullStackSignal.ts @@ -137,6 +137,7 @@ export abstract class FullStackSignal extends Signal { this.subscribe((v) => { if (!paused) { this.#pending.value = true; + this.#error.value = undefined; this.server .update({ id: nanoid(), diff --git a/packages/ts/react-signals/test/FullStackSignal.spec.tsx b/packages/ts/react-signals/test/FullStackSignal.spec.tsx index e9b32906fd..344bd81705 100644 --- a/packages/ts/react-signals/test/FullStackSignal.spec.tsx +++ b/packages/ts/react-signals/test/FullStackSignal.spec.tsx @@ -142,15 +142,21 @@ describe('@vaadin/hilla-react-signals', () => { // Waiting for the ConnectionClient#call promise to resolve. await nextFrame(); - expect(signal.error).to.have.property('value', error); + expect(signal.error).to.be.like({ value: error }); + + // No error after the correct update + client.call.resolves(); + signal.value = 50; + await nextFrame(); + expect(signal.error).to.be.like({ value: undefined }); }); it('should provide a way to access the pending state', async () => { - expect(signal.pending).to.have.property('value', false); + expect(signal.pending).to.be.like({ value: false }); signal.value = 42; - expect(signal.pending).to.have.property('value', true); + expect(signal.pending).to.be.like({ value: true }); await nextFrame(); - expect(signal.pending).to.have.property('value', false); + expect(signal.pending).to.be.like({ value: false }); }); it('should provide an internal server subscription', () => {