add wasm rulesengine by cbrady · Pull Request #59 · SchematicHQ/schematic-node

cbrady · 2025-12-02T16:26:57Z

No description provided.

src/cache/redis.ts

src/rules-engine.ts

src/cache/redis.ts

bpapillon · 2026-01-22T19:57:01Z

src/cache/redis.ts

+    this.keyPrefix = options.keyPrefix || 'schematic:';
+
+    // Dynamically import Redis to avoid requiring it if not used
+    this.initRedisClient(options);


Is it okay that we don't await this? Are we concerned that, when redis is used, there could be calls made before the redis client is ready?

yeah this is how the other datastream versions work kind of fire and let it do its thing

src/cache/redis.ts

src/datastream/datastream-client.ts

src/rules-engine.ts

src/wrapper.ts

src/datastream/datastream-client.ts

bpapillon

a few more small comments. also, probably we don't want to check in package-lock.json

src/datastream/datastream-client.ts

src/cache/redis.ts

bpapillon · 2026-02-05T15:41:00Z

src/datastream/datastream-client.test.ts

+  sendMessage: jest.fn().mockResolvedValue(undefined),
+};
+
+jest.mock('./client', () => {


I would have thought the path here would be "./datastrem-client". I don't know that much about jest mocking - why does this work with an incorrect path?

bpapillon · 2026-02-05T15:41:25Z

src/datastream/datastream-client.test.ts

+import * as Schematic from '../api/types';
+
+// Mock DatastreamWSClient
+const mockDatastreamWSClientInstanceInstance = {


name should maybe be "mockDatastreamWSClientInstance" here?

src/datastream/datastream-client.ts

bpapillon · 2026-02-05T15:43:36Z

src/datastream/datastream-client.test.ts

+    await messageHandler({}, companyErrorMessage);
+
+    // Verify warning was logged but no error event was emitted
+    expect(mockLogger.warn).toHaveBeenCalledWith({}, 'DataStream error received: Company not found');


Shouldn't these tests be failing now after the logger interface change from the last review pass?

I wonder if these tests are being skipped for some reason

bpapillon · 2026-02-05T15:47:30Z

src/datastream/datastream-client.ts

+    try {
+      const response = await fetch(this.replicatorHealthURL, {
+        method: 'GET',
+        // @ts-ignore - timeout is supported in newer Node.js versions


I was looking into this ts-ignore, and I'm. not sure I understand this comment- by my understanding is that the AbortController pattern is recommended regardless of node version

https://betterstack.com/community/guides/scaling-nodejs/nodejs-timeouts/

src/datastream/websocket-client.ts

bpapillon · 2026-02-05T15:54:37Z

src/datastream/datastream-client.ts

+      if (flag?.key) {
+        const cacheKey = this.flagCacheKey(flag.key);
+        try {
+          await this.flagsCacheProvider.set(cacheKey, flag);


Any thought of using Promise.all here?

src/wrapper.ts

cbrady self-assigned this Dec 2, 2025

cbrady force-pushed the chris/sch-5303-datastream-replicator-in-the-typescript-node-sdk branch from 2ed93f5 to bfc0dad Compare December 2, 2025 17:19

cbrady marked this pull request as ready for review December 11, 2025 00:50

cbrady requested a review from a team as a code owner December 11, 2025 00:50

cbrady force-pushed the chris/sch-5303-datastream-replicator-in-the-typescript-node-sdk branch from 8a59ba9 to 81b4c3b Compare December 23, 2025 22:24

bpapillon reviewed Jan 16, 2026

View reviewed changes

src/cache/redis.ts Outdated Show resolved Hide resolved

src/rules-engine.ts Outdated Show resolved Hide resolved

src/cache/redis.ts Outdated Show resolved Hide resolved

cbrady added 11 commits January 22, 2026 09:37

add wasm rulesengine

cc417a9

add datastream ws client

d827524

add redis cache option and refactor cache functionality

713d73c

add datastream and replicator functionalty

13df222

add necessary wasm files

82b97c0

remove unused imports

4fe7f5a

cleaner way of dynamically importing redis

cdc5032

remove logging from redis client

d8f356a

update to use default value functiionality

f6d71c0

error handling around cache operations

7d1a14c

check cache first before falling back

932cb49

cbrady force-pushed the chris/sch-5303-datastream-replicator-in-the-typescript-node-sdk branch from 81b4c3b to 932cb49 Compare January 22, 2026 17:28

bpapillon requested changes Jan 22, 2026

View reviewed changes

cbrady added 7 commits January 22, 2026 17:22

remove clear from cache providers

461cc44

use rulesengine check flag result for check flag result

b070a1f

ensure redis is connected before performing operations

afbefae

use scan when bulk deleting keys

3e265af

no need to double parse wasm responses

6583a04

rework how we turn on datastream

6f38c75

catch datastream init errors

553abe2

bpapillon requested changes Jan 28, 2026

View reviewed changes

src/datastream/datastream-client.ts Outdated Show resolved Hide resolved

src/cache/redis.ts Outdated Show resolved Hide resolved

cbrady added 3 commits January 28, 2026 11:09

remove package-lock file

113020f

use global logger

d161c8c

when deleting missing keys allow for more specific pattern

a0828cf

cbrady force-pushed the chris/sch-5303-datastream-replicator-in-the-typescript-node-sdk branch from 10d4f05 to a0828cf Compare January 28, 2026 20:00

bpapillon requested changes Feb 5, 2026

View reviewed changes

refactor to address comments

3da60d8

Conversation

cbrady commented Dec 2, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bpapillon left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants