Skip to content

Commit 23ce433

Browse files
committed
feat(core)!: track node statuses in core
1 parent 0847430 commit 23ce433

File tree

8 files changed

+252
-112
lines changed

8 files changed

+252
-112
lines changed

packages/core/src/_exports/index.ts

+2-1
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,8 @@ export {
3232
releaseChannel,
3333
} from '../comlink/controller/comlinkControllerStore'
3434
export type {ComlinkNodeState} from '../comlink/node/comlinkNodeStore'
35-
export {getOrCreateNode, releaseNode} from '../comlink/node/comlinkNodeStore'
35+
export {getNodeState} from '../comlink/node/getNodeState'
36+
export {type NodeState} from '../comlink/node/getNodeState'
3637
export {type FrameMessage, type WindowMessage} from '../comlink/types'
3738
export {type AuthConfig, type AuthProvider} from '../config/authConfig'
3839
export {

packages/core/src/comlink/node/actions/getOrCreateNode.test.ts

+43-3
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ vi.mock('@sanity/comlink', () => ({
1212
createNode: vi.fn(() => ({
1313
start: vi.fn(),
1414
stop: vi.fn(),
15+
onStatus: vi.fn(),
1516
})),
1617
}))
1718

@@ -32,9 +33,9 @@ describe('getOrCreateNode', () => {
3233
}
3334

3435
beforeEach(() => {
35-
mockNode = {start: vi.fn(), stop: vi.fn()}
36+
mockNode = {start: vi.fn(), stop: vi.fn(), onStatus: vi.fn()}
3637
vi.mocked(comlink.createNode).mockReturnValue(mockNode as Node<WindowMessage, FrameMessage>)
37-
state = createStoreState<ComlinkNodeState>({nodes: new Map()})
38+
state = createStoreState<ComlinkNodeState>({nodes: new Map(), subscriptions: new Map()})
3839
vi.clearAllMocks()
3940
})
4041

@@ -45,7 +46,7 @@ describe('getOrCreateNode', () => {
4546
expect(node.start).toHaveBeenCalled()
4647
})
4748

48-
it('sshould store the node in nodeStore', () => {
49+
it('should store the node in nodeStore', () => {
4950
const node = getOrCreateNode({state, instance}, nodeConfig)
5051

5152
expect(getOrCreateNode({state, instance}, nodeConfig)).toBe(node)
@@ -64,4 +65,43 @@ describe('getOrCreateNode', () => {
6465
),
6566
).toThrow('Node "test-node" already exists with different options')
6667
})
68+
69+
it('should subscribe to status changes and update state', () => {
70+
let statusCallback: ((status: string) => void) | undefined
71+
const statusUnsubMock = vi.fn()
72+
mockNode.onStatus = vi.fn((cb) => {
73+
statusCallback = cb
74+
return statusUnsubMock
75+
})
76+
77+
getOrCreateNode({state, instance}, nodeConfig)
78+
79+
expect(mockNode.onStatus).toHaveBeenCalled()
80+
expect(typeof statusCallback).toBe('function')
81+
expect(state.get().nodes.get(nodeConfig.name)?.statusUnsub).toBe(statusUnsubMock)
82+
83+
statusCallback?.('connected')
84+
85+
expect(state.get().nodes.get(nodeConfig.name)?.status).toBe('connected')
86+
})
87+
88+
it('should not update state if node entry is missing when status changes', () => {
89+
let statusCallback: ((status: string) => void) | undefined
90+
const statusUnsubMock = vi.fn()
91+
mockNode.onStatus = vi.fn((cb) => {
92+
statusCallback = cb
93+
return statusUnsubMock
94+
})
95+
96+
getOrCreateNode({state, instance}, nodeConfig)
97+
98+
// Remove the node entry before triggering the status callback
99+
state.get().nodes.delete(nodeConfig.name)
100+
101+
// Simulate a status change
102+
statusCallback?.('connected')
103+
104+
// Assert: node entry is still missing, so no update occurred
105+
expect(state.get().nodes.has(nodeConfig.name)).toBe(false)
106+
})
67107
})

packages/core/src/comlink/node/actions/getOrCreateNode.ts

+24-14
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,6 @@ import {type StoreContext} from '../../../store/defineStore'
55
import {type FrameMessage, type WindowMessage} from '../../types'
66
import {type ComlinkNodeState} from '../comlinkNodeStore'
77

8-
/**
9-
* Retrieve or create a node to be used for communication between
10-
* an application and the controller -- specifically, a node should
11-
* be created within a frame / window to communicate with the controller.
12-
* @public
13-
*/
148
export const getOrCreateNode = (
159
{state}: StoreContext<ComlinkNodeState>,
1610
options: NodeInput,
@@ -24,21 +18,37 @@ export const getOrCreateNode = (
2418
throw new Error(`Node "${options.name}" already exists with different options`)
2519
}
2620

27-
state.set('incrementNodeRefCount', {
28-
nodes: new Map(nodes).set(options.name, {
29-
...existing,
30-
refCount: existing.refCount + 1,
31-
}),
32-
})
33-
3421
existing.node.start()
3522
return existing.node
3623
}
3724

3825
const node: Node<WindowMessage, FrameMessage> = createNode(options)
3926
node.start()
4027

41-
nodes.set(options.name, {node, options, refCount: 1})
28+
// Subscribe to status changes
29+
const statusUnsub = node.onStatus((status) => {
30+
const currentNodes = state.get().nodes
31+
const currentEntry = currentNodes.get(options.name)
32+
if (!currentEntry) return
33+
const updatedEntry = {
34+
...currentEntry,
35+
status,
36+
statusUnsub: currentEntry.statusUnsub,
37+
}
38+
state.set('updateNodeStatus', {
39+
nodes: new Map(currentNodes).set(options.name, updatedEntry),
40+
})
41+
})
42+
43+
// Set up initial entry with status, error, and statusUnsub
44+
const entry = {
45+
node,
46+
options,
47+
status: 'idle' as const,
48+
statusUnsub,
49+
}
50+
51+
nodes.set(options.name, entry)
4252

4353
state.set('createNode', {nodes})
4454

packages/core/src/comlink/node/actions/releaseNode.test.ts

+8-73
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import {beforeEach, describe, expect, it, vi} from 'vitest'
44
import {createSanityInstance, type SanityInstance} from '../../../store/createSanityInstance'
55
import {createStoreState} from '../../../store/createStoreState'
66
import {type FrameMessage, type WindowMessage} from '../../types'
7-
import {type ComlinkNodeState, type NodeEntry} from '../comlinkNodeStore'
7+
import {type ComlinkNodeState} from '../comlinkNodeStore'
88
import {releaseNode} from './releaseNode'
99

1010
const nodeConfig = {
@@ -18,15 +18,16 @@ describe('releaseNode', () => {
1818
let mockNode: Partial<Node<WindowMessage, FrameMessage>> & {
1919
start: ReturnType<typeof vi.fn>
2020
stop: ReturnType<typeof vi.fn>
21+
onStatus: ReturnType<typeof vi.fn>
2122
}
2223

2324
beforeEach(() => {
2425
instance = createSanityInstance({
2526
projectId: 'test-project-id',
2627
dataset: 'test-dataset',
2728
})
28-
mockNode = {start: vi.fn(), stop: vi.fn()}
29-
state = createStoreState<ComlinkNodeState>({nodes: new Map()})
29+
mockNode = {start: vi.fn(), stop: vi.fn(), onStatus: vi.fn()}
30+
state = createStoreState<ComlinkNodeState>({nodes: new Map(), subscriptions: new Map()})
3031
vi.clearAllMocks()
3132
})
3233

@@ -40,7 +41,6 @@ describe('releaseNode', () => {
4041
nodes.set('test-node', {
4142
node: mockNode as Node<WindowMessage, FrameMessage>,
4243
options: nodeConfig,
43-
refCount: 1,
4444
})
4545
state.set('setup', {nodes})
4646

@@ -54,83 +54,18 @@ describe('releaseNode', () => {
5454
expect(state.get().nodes.has('test-node')).toBe(false)
5555
})
5656

57-
it('should not stop the node if refCount is still above 0', () => {
58-
// Create a node twice to increment refCount
57+
it('should call statusUnsub if present when releasing node', () => {
58+
const statusUnsub = vi.fn()
5959
const nodes = new Map()
6060
nodes.set('test-node', {
6161
node: mockNode as Node<WindowMessage, FrameMessage>,
6262
options: nodeConfig,
63-
refCount: 2,
63+
statusUnsub,
6464
})
6565
state.set('setup', {nodes})
6666

67-
// Release once
6867
releaseNode({state, instance}, 'test-node')
6968

70-
// Node should not be stopped
71-
expect(mockNode.stop).not.toHaveBeenCalled()
72-
73-
// Verify refCount is 1
74-
const nodeEntry = state.get().nodes.get('test-node') as NodeEntry
75-
expect(nodeEntry?.refCount).toBe(1)
76-
})
77-
78-
it('should handle multiple releases gracefully', () => {
79-
// Set up a node in the state
80-
const nodes = new Map()
81-
nodes.set('test-node', {
82-
node: mockNode as Node<WindowMessage, FrameMessage>,
83-
options: nodeConfig,
84-
refCount: 1,
85-
})
86-
state.set('setup', {nodes})
87-
88-
// Release multiple times
89-
releaseNode({state, instance}, 'test-node')
90-
releaseNode({state, instance}, 'test-node')
91-
releaseNode({state, instance}, 'test-node')
92-
93-
// Verify node is removed after first release
94-
expect(state.get().nodes.has('test-node')).toBe(false)
95-
// Stop should be called exactly once
96-
expect(mockNode.stop).toHaveBeenCalledTimes(1)
97-
})
98-
99-
it('should handle releasing non-existent nodes', () => {
100-
// Should not throw when releasing non-existent node
101-
expect(() => releaseNode({state, instance}, 'non-existent')).not.toThrow()
102-
})
103-
104-
it('should maintain correct state after complex operations', () => {
105-
// Set up a node with refCount = 3
106-
const nodes = new Map()
107-
nodes.set('test-node', {
108-
node: mockNode as Node<WindowMessage, FrameMessage>,
109-
options: nodeConfig,
110-
refCount: 3,
111-
})
112-
state.set('setup', {nodes})
113-
114-
// Initial refCount should be 3
115-
let nodeEntry = state.get().nodes.get('test-node') as NodeEntry
116-
expect(nodeEntry?.refCount).toBe(3)
117-
118-
// Release twice
119-
releaseNode({state, instance}, 'test-node')
120-
releaseNode({state, instance}, 'test-node')
121-
122-
nodeEntry = state.get().nodes.get('test-node') as NodeEntry
123-
expect(nodeEntry?.refCount).toBe(1)
124-
125-
// Verify node hasn't been stopped yet
126-
expect(mockNode.stop).not.toHaveBeenCalled()
127-
128-
// Release final reference
129-
releaseNode({state, instance}, 'test-node')
130-
131-
// Verify node was stopped
132-
expect(mockNode.stop).toHaveBeenCalled()
133-
134-
expect(state.get().nodes.has('test-node')).toBe(false)
69+
expect(statusUnsub).toHaveBeenCalled()
13570
})
13671
})
Original file line numberDiff line numberDiff line change
@@ -1,30 +1,17 @@
11
import {type StoreContext} from '../../../store/defineStore'
22
import {type ComlinkNodeState} from '../comlinkNodeStore'
33

4-
/**
5-
* Release a node that was previously created with getOrCreateNode.
6-
* @public
7-
*/
84
export const releaseNode = ({state}: StoreContext<ComlinkNodeState>, name: string): void => {
95
const nodes = state.get().nodes
106
const existing = nodes.get(name)
117

128
if (existing) {
13-
const newRefCount = existing.refCount - 1
14-
15-
if (newRefCount <= 0) {
16-
existing.node.stop()
17-
nodes.delete(name)
18-
state.set('removeNode', {nodes})
19-
return
9+
if (existing.statusUnsub) {
10+
existing.statusUnsub()
2011
}
21-
22-
state.set('decrementNodeRefCount', {
23-
nodes: new Map(nodes).set(name, {
24-
...existing,
25-
refCount: newRefCount,
26-
}),
27-
})
12+
existing.node.stop()
13+
nodes.delete(name)
14+
state.set('removeNode', {nodes})
2815
return
2916
}
3017
}

packages/core/src/comlink/node/comlinkNodeStore.ts

+7-3
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import {type Node, type NodeInput} from '@sanity/comlink'
1+
import {type Node, type NodeInput, type Status} from '@sanity/comlink'
22

33
import {bindActionGlobally} from '../../store/createActionBinder'
44
import {defineStore} from '../../store/defineStore'
@@ -14,8 +14,9 @@ export interface NodeEntry {
1414
node: Node<WindowMessage, FrameMessage>
1515
// we store options to ensure that channels remain as unique / consistent as possible
1616
options: NodeInput
17-
// we store refCount to ensure nodes are running only as long as they are in use
18-
refCount: number
17+
// status of the node connection
18+
status: Status
19+
statusUnsub?: () => void
1920
}
2021

2122
/**
@@ -24,12 +25,15 @@ export interface NodeEntry {
2425
*/
2526
export interface ComlinkNodeState {
2627
nodes: Map<string, NodeEntry>
28+
// Map of node name to set of active subscriber symbols
29+
subscriptions: Map<string, Set<symbol>>
2730
}
2831

2932
export const comlinkNodeStore = defineStore<ComlinkNodeState>({
3033
name: 'nodeStore',
3134
getInitialState: () => ({
3235
nodes: new Map(),
36+
subscriptions: new Map(),
3337
}),
3438

3539
initialize({state}) {

0 commit comments

Comments
 (0)