Skip to content

Commit cb2fb5a

Browse files
committed
Refactor transport factory from object to function
1 parent c198459 commit cb2fb5a

File tree

4 files changed

+26
-36
lines changed

4 files changed

+26
-36
lines changed

src/core/Ros.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ export default class Ros extends EventEmitter<
5959

6060
constructor({
6161
url,
62-
transportFactory = new WebSocketTransportFactory(),
62+
transportFactory = WebSocketTransportFactory,
6363
}: {
6464
/**
6565
* The rosbridge server URL. Can be specified later with `connect`.
@@ -91,7 +91,7 @@ export default class Ros extends EventEmitter<
9191
return; // Already connected
9292
}
9393

94-
const transport = await this.transportFactory.createTransport(url);
94+
const transport = await this.transportFactory(url);
9595
this.transport = transport;
9696

9797
transport.on("open", (event: TransportEvent) => {

src/core/Transport.ts

Lines changed: 15 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -39,9 +39,7 @@ export interface ITransport {
3939
isClosed(): boolean;
4040
}
4141

42-
export interface ITransportFactory {
43-
createTransport(url: string): Promise<ITransport>;
44-
}
42+
export type ITransportFactory = (url: string) => Promise<ITransport>;
4543

4644
export abstract class AbstractTransport
4745
extends EventEmitter<{
@@ -363,19 +361,19 @@ export class WsWebSocketTransport extends AbstractTransport {
363361
* @see https://developer.mozilla.org/en-US/docs/Web/API/WebSocket
364362
* @see https://github.com/websockets/ws
365363
*/
366-
export class WebSocketTransportFactory implements ITransportFactory {
367-
public async createTransport(url: string): Promise<ITransport> {
368-
// Browsers, Deno, Bun, and Node 22+ support WebSockets natively
369-
if (typeof WebSocket === "function") {
370-
const socket = new WebSocket(url);
371-
socket.binaryType = "arraybuffer";
372-
return new NativeWebSocketTransport(socket);
373-
}
374-
375-
// If in Node.js, import ws to replace WebSocket API
376-
const ws = await import("ws");
377-
const socket = new ws.WebSocket(url);
364+
export const WebSocketTransportFactory: ITransportFactory = async (
365+
url: string,
366+
): Promise<ITransport> => {
367+
// Browsers, Deno, Bun, and Node 22+ support WebSockets natively
368+
if (typeof WebSocket === "function") {
369+
const socket = new WebSocket(url);
378370
socket.binaryType = "arraybuffer";
379-
return new WsWebSocketTransport(socket);
371+
return new NativeWebSocketTransport(socket);
380372
}
381-
}
373+
374+
// If in Node.js, import ws to replace WebSocket API
375+
const ws = await import("ws");
376+
const socket = new ws.WebSocket(url);
377+
socket.binaryType = "arraybuffer";
378+
return new WsWebSocketTransport(socket);
379+
};

test/ros.test.ts

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -98,9 +98,7 @@ describe("Ros", function () {
9898
isClosed: vi.fn(),
9999
};
100100

101-
mockTransportFactory = {
102-
createTransport: vi.fn().mockReturnValue(mockTransport),
103-
};
101+
mockTransportFactory = vi.fn().mockReturnValue(mockTransport);
104102
});
105103

106104
afterEach(() => {
@@ -113,7 +111,7 @@ describe("Ros", function () {
113111
const ros = new Ros();
114112

115113
// @ts-expect-error -- spying on private property
116-
expect(ros.transportFactory).toBeInstanceOf(WebSocketTransportFactory);
114+
expect(ros.transportFactory).toBe(WebSocketTransportFactory);
117115
});
118116

119117
it("creates a transport using a factory function", async () => {
@@ -123,9 +121,7 @@ describe("Ros", function () {
123121

124122
await ros.connect(mockRosUrl);
125123

126-
expect(mockTransportFactory.createTransport).toHaveBeenCalledWith(
127-
mockRosUrl,
128-
);
124+
expect(mockTransportFactory).toHaveBeenCalledWith(mockRosUrl);
129125
});
130126

131127
it("does not create a new transport if the socket is not closed", async () => {
@@ -135,13 +131,13 @@ describe("Ros", function () {
135131

136132
// always creates a new transport the first time
137133
await ros.connect(mockRosUrl);
138-
expect(mockTransportFactory.createTransport).toHaveBeenCalledTimes(1);
134+
expect(mockTransportFactory).toHaveBeenCalledTimes(1);
139135

140136
vi.clearAllMocks();
141137

142138
// socket is not closed so no new transport is created
143139
await ros.connect(mockRosUrl);
144-
expect(mockTransportFactory.createTransport).toHaveBeenCalledTimes(0);
140+
expect(mockTransportFactory).toHaveBeenCalledTimes(0);
145141
});
146142

147143
it("creates a new transport if the socket is closed", async () => {
@@ -151,15 +147,15 @@ describe("Ros", function () {
151147

152148
// always creates a new transport the first time
153149
await ros.connect(mockRosUrl);
154-
expect(mockTransportFactory.createTransport).toHaveBeenCalledTimes(1);
150+
expect(mockTransportFactory).toHaveBeenCalledTimes(1);
155151

156152
vi.clearAllMocks();
157153

158154
mockTransport.close();
159155

160156
// socket is closed so a new transport is created
161157
await ros.connect(mockRosUrl);
162-
expect(mockTransportFactory.createTransport).toHaveBeenCalledTimes(1);
158+
expect(mockTransportFactory).toHaveBeenCalledTimes(1);
163159
});
164160
});
165161

test/transport.test.ts

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -650,9 +650,7 @@ describe("Transport", () => {
650650
vi.stubGlobal("WebSocket", WebSocket);
651651
expect(typeof WebSocket).toBe("function");
652652

653-
const factory = new WebSocketTransportFactory();
654-
655-
const transport = await factory.createTransport("ws://localhost:9090");
653+
const transport = await WebSocketTransportFactory("ws://localhost:9090");
656654

657655
expect(transport).toBeInstanceOf(NativeWebSocketTransport);
658656
});
@@ -661,9 +659,7 @@ describe("Transport", () => {
661659
vi.stubGlobal("WebSocket", undefined);
662660
expect(typeof WebSocket).toBe("undefined");
663661

664-
const factory = new WebSocketTransportFactory();
665-
666-
const transport = await factory.createTransport("ws://localhost:9090");
662+
const transport = await WebSocketTransportFactory("ws://localhost:9090");
667663

668664
expect(transport).toBeInstanceOf(WsWebSocketTransport);
669665
});

0 commit comments

Comments
 (0)