Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion packages/core/source/receivers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,8 @@ export {
type RemoteReceiverComment,
} from './receivers/RemoteReceiver.ts';
export {DOMRemoteReceiver} from './receivers/DOMRemoteReceiver.ts';
export type {RemoteReceiverOptions} from './receivers/shared.ts';
export {THROW_DEFAULT} from './receivers/shared.ts';
export type {
RemoteReceiverOptions,
MissingImplementationContext,
} from './receivers/shared.ts';
20 changes: 19 additions & 1 deletion packages/core/source/receivers/RemoteReceiver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import type {
RemoteElementSerialization,
RemoteNodeSerialization,
} from '../types.ts';
import type {RemoteReceiverOptions} from './shared.ts';
import {THROW_DEFAULT, type RemoteReceiverOptions} from './shared.ts';

/**
* Represents a text node of a remote tree in a plain JavaScript format, with
Expand Down Expand Up @@ -139,6 +139,7 @@ export class RemoteReceiver {
retain,
release,
methods,
onMissingImplementationError,
}: RemoteReceiverOptions & {
/**
* A set of [remote methods](https://github.com/Shopify/remote-dom/blob/main/packages/core#remotemethods)
Expand All @@ -155,6 +156,23 @@ export class RemoteReceiver {
const implementationMethod = implementation?.[method];

if (typeof implementationMethod !== 'function') {
if (onMissingImplementationError) {
const node = attached.get(id);
const element =
node && 'type' in node && node.type === NODE_TYPE_ELEMENT
? (node as RemoteReceiverElement).element
: undefined;

const result = onMissingImplementationError({
id,
method,
args,
element,
});

if (result !== THROW_DEFAULT) return result;
}

throw new Error(
`Node ${id} does not implement the ${method}() method`,
);
Expand Down
36 changes: 36 additions & 0 deletions packages/core/source/receivers/shared.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,28 @@
/**
* Return this sentinel from error handlers to fall through to the default
* error case.
*/
export const THROW_DEFAULT: unique symbol = Symbol.for(
'remote-dom.throw-default',
);
Comment on lines +5 to +7
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Throwing is the default in this case but a default is a default, right?

Suggested change
export const THROW_DEFAULT: unique symbol = Symbol.for(
'remote-dom.throw-default',
);
export const DEFAULT: unique symbol = Symbol.for(
'remote-dom.default',
);


/**
* Context passed to the `onMissingImplementationError` callback.
*/
export interface MissingImplementationContext {
/** The ID of the node the method was called on. */
id: string;
/** The name of the method that was called. */
method: string;
/** The arguments passed to the method call. */
args: readonly unknown[];
/**
* The element type of the node (e.g. `'s-modal'`, `'s-sheet'`), or
* `undefined` if the node is not an element or is not found in the tree.
*/
element: string | undefined;
}

/**
* Options that are useful for all remote receiver implementations. All of the
* receivers in `@remote-dom/core/receivers` accept these options.
Expand All @@ -20,4 +45,15 @@ export interface RemoteReceiverOptions {
* by the host implementation.
*/
release?(value: any): void;

/**
* Called when `connection.call()` is invoked on a node with no registered
* implementation for the requested method. By default, an error is thrown.
* No-ops on receivers that don't implement `implement()` (`DOMRemoteReceiver`).
*
* Return a value to use as the method's return value. Return
* `THROW_DEFAULT` to fall through to the default error. Throw to
* replace the error entirely.
*/
onMissingImplementationError?(context: MissingImplementationContext): unknown;
}
85 changes: 77 additions & 8 deletions packages/core/source/tests/elements.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import {
RemoteReceiver,
type RemoteReceiverElement,
} from '../receivers/RemoteReceiver.ts';
import {THROW_DEFAULT} from '../receivers/shared.ts';
import {
MUTATION_TYPE_UPDATE_PROPERTY,
UPDATE_PROPERTY_TYPE_PROPERTY,
Expand Down Expand Up @@ -1294,6 +1295,68 @@ describe('RemoteElement', () => {
expect(result).toBe(`Hello ${name}!`);
expect(spy).toHaveBeenCalledWith(name);
});

it('throws when calling a method with no implementation', () => {
class HelloElement extends RemoteElement<{}, {greet(): void}> {
greet(name: string) {
return this.callRemoteMethod('greet', name);
}
}

const {root} = createAndConnectRemoteRootElement();

const element = new HelloElement();
root.append(element);

expect(() => {
element.greet('Winston');
}).toThrow('does not implement the greet() method');
});

it('calls onMissingImplementationError with context instead of throwing when provided', () => {
class HelloElement extends RemoteElement<{}, {greet(): void}> {
greet(name: string) {
return this.callRemoteMethod('greet', name);
}
}

const onMissingImplementationError = vi.fn(() => 'fallback');
const {root} = createAndConnectRemoteRootElement({
onMissingImplementationError,
});

const element = new HelloElement();
root.append(element);

const result = element.greet('Winston');

expect(onMissingImplementationError).toHaveBeenCalledWith(
expect.objectContaining({
method: 'greet',
args: ['Winston'],
}),
);
expect(result).toBe('fallback');
});

it('falls through to default error when THROW_DEFAULT is returned', () => {
class HelloElement extends RemoteElement<{}, {greet(): void}> {
greet(name: string) {
return this.callRemoteMethod('greet', name);
}
}

const {root} = createAndConnectRemoteRootElement({
onMissingImplementationError: () => THROW_DEFAULT,
});

const element = new HelloElement();
root.append(element);

expect(() => {
element.greet('Winston');
}).toThrow('does not implement the greet() method');
});
});
});

Expand All @@ -1304,25 +1367,29 @@ class TestRemoteReceiver
'root' | 'connection' | 'get' | 'implement' | 'subscribe'
>
{
readonly #receiver = new RemoteReceiver();
readonly #receiver: RemoteReceiver;
readonly connection: RemoteReceiver['connection'] &
MockedObject<RemoteReceiver['connection']>;

get root() {
return this.#receiver.root;
}

constructor() {
get: RemoteReceiver['get'];
implement: RemoteReceiver['implement'];
subscribe: RemoteReceiver['subscribe'];

constructor(options?: ConstructorParameters<typeof RemoteReceiver>[0]) {
this.#receiver = new RemoteReceiver(options);
const {connection} = this.#receiver;
this.connection = {
mutate: vi.fn(connection.mutate),
call: vi.fn(connection.call),
};
this.get = this.#receiver.get.bind(this.#receiver);
this.implement = this.#receiver.implement.bind(this.#receiver);
this.subscribe = this.#receiver.subscribe.bind(this.#receiver);
}

get: RemoteReceiver['get'] = this.#receiver.get.bind(this.#receiver);
implement = this.#receiver.implement.bind(this.#receiver);
subscribe = this.#receiver.subscribe.bind(this.#receiver);
}

function createAndConnectRemoteElement<
Expand Down Expand Up @@ -1355,9 +1422,11 @@ function createElementFromConstructor<
return element;
}

function createAndConnectRemoteRootElement() {
function createAndConnectRemoteRootElement(
options?: ConstructorParameters<typeof RemoteReceiver>[0],
) {
const root = createRemoteRootElement();
const receiver = new TestRemoteReceiver();
const receiver = new TestRemoteReceiver(options);
root.connect(receiver.connection);
document.body.append(root);
return {root, receiver};
Expand Down
26 changes: 24 additions & 2 deletions packages/signals/source/SignalRemoteReceiver.ts
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change doesn't seem to have any test coverage. I guess that's because there's no suite for this at all?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can backfill some tests here for sure.

Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,10 @@ import {
type RemoteCommentSerialization,
type RemoteElementSerialization,
} from '@remote-dom/core';
import type {RemoteReceiverOptions} from '@remote-dom/core/receivers';
import {
THROW_DEFAULT,
type RemoteReceiverOptions,
} from '@remote-dom/core/receivers';

/**
* Represents a text node of a remote tree in a plain JavaScript format, with
Expand Down Expand Up @@ -142,7 +145,11 @@ export class SignalRemoteReceiver {
Record<string, (...args: unknown[]) => unknown>
>();

constructor({retain, release}: RemoteReceiverOptions = {}) {
constructor({
retain,
release,
onMissingImplementationError,
}: RemoteReceiverOptions = {}) {
const {attached, parents} = this;

const baseConnection = createRemoteConnection({
Expand All @@ -151,6 +158,21 @@ export class SignalRemoteReceiver {
const implementationMethod = implementation?.[method];

if (typeof implementationMethod !== 'function') {
if (onMissingImplementationError) {
const node = attached.get(id);
const element =
node && 'element' in node ? node.element : undefined;

const result = onMissingImplementationError({
id,
method,
args,
element,
});

if (result !== THROW_DEFAULT) return result;
}

throw new Error(
`Node ${id} does not implement the ${method}() method`,
);
Expand Down
Loading