Skip to content

Commit 143f417

Browse files
authored
Merge pull request #3102 from tearf001/fix-issue-2937-weakmap
feat(test): add vitest setup and widget container tests
2 parents d68d80a + ed81728 commit 143f417

File tree

6 files changed

+1082
-8
lines changed

6 files changed

+1082
-8
lines changed
Lines changed: 141 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,141 @@
1+
import { describe, it, expect, beforeEach, vi } from 'vitest';
2+
import { gridWidgetContainersMap } from './grid-stack-render-provider';
3+
4+
// Mock GridStack type
5+
class MockGridStack {
6+
el: HTMLElement;
7+
constructor() {
8+
this.el = document.createElement('div');
9+
}
10+
}
11+
12+
describe('GridStackRenderProvider', () => {
13+
beforeEach(() => {
14+
// Clear the WeakMap before each test
15+
gridWidgetContainersMap.constructor.prototype.clear?.call(gridWidgetContainersMap);
16+
});
17+
18+
it('should store widget containers in WeakMap for each grid instance', () => {
19+
// Mock grid instances
20+
const grid1 = new MockGridStack() as any;
21+
const grid2 = new MockGridStack() as any;
22+
const widget1 = { id: '1', grid: grid1 };
23+
const widget2 = { id: '2', grid: grid2 };
24+
const element1 = document.createElement('div');
25+
const element2 = document.createElement('div');
26+
27+
// Simulate renderCB
28+
const renderCB = (element, widget) => {
29+
if (widget.id && widget.grid) {
30+
// Get or create the widget container map for this grid instance
31+
let containers = gridWidgetContainersMap.get(widget.grid);
32+
if (!containers) {
33+
containers = new Map();
34+
gridWidgetContainersMap.set(widget.grid, containers);
35+
}
36+
containers.set(widget.id, element);
37+
}
38+
};
39+
40+
renderCB(element1, widget1);
41+
renderCB(element2, widget2);
42+
43+
const containers1 = gridWidgetContainersMap.get(grid1);
44+
const containers2 = gridWidgetContainersMap.get(grid2);
45+
46+
expect(containers1?.get('1')).toBe(element1);
47+
expect(containers2?.get('2')).toBe(element2);
48+
});
49+
50+
it('should not have containers for different grid instances mixed up', () => {
51+
const grid1 = new MockGridStack() as any;
52+
const grid2 = new MockGridStack() as any;
53+
const widget1 = { id: '1', grid: grid1 };
54+
const widget2 = { id: '2', grid: grid1 };
55+
const widget3 = { id: '3', grid: grid2 };
56+
const element1 = document.createElement('div');
57+
const element2 = document.createElement('div');
58+
const element3 = document.createElement('div');
59+
60+
// Simulate renderCB
61+
const renderCB = (element: HTMLElement, widget: any) => {
62+
if (widget.id && widget.grid) {
63+
let containers = gridWidgetContainersMap.get(widget.grid);
64+
if (!containers) {
65+
containers = new Map();
66+
gridWidgetContainersMap.set(widget.grid, containers);
67+
}
68+
containers.set(widget.id, element);
69+
}
70+
};
71+
72+
renderCB(element1, widget1);
73+
renderCB(element2, widget2);
74+
renderCB(element3, widget3);
75+
76+
const containers1 = gridWidgetContainersMap.get(grid1);
77+
const containers2 = gridWidgetContainersMap.get(grid2);
78+
79+
// Grid1 should have widgets 1 and 2
80+
expect(containers1?.size).toBe(2);
81+
expect(containers1?.get('1')).toBe(element1);
82+
expect(containers1?.get('2')).toBe(element2);
83+
expect(containers1?.get('3')).toBeUndefined();
84+
85+
// Grid2 should only have widget 3
86+
expect(containers2?.size).toBe(1);
87+
expect(containers2?.get('3')).toBe(element3);
88+
expect(containers2?.get('1')).toBeUndefined();
89+
expect(containers2?.get('2')).toBeUndefined();
90+
});
91+
92+
it('should clean up when grid instance is deleted from WeakMap', () => {
93+
const grid = new MockGridStack() as any;
94+
const widget = { id: '1', grid };
95+
const element = document.createElement('div');
96+
97+
// Add to WeakMap
98+
const containers = new Map<string, HTMLElement>();
99+
containers.set(widget.id, element);
100+
gridWidgetContainersMap.set(grid, containers);
101+
102+
// Verify it exists
103+
expect(gridWidgetContainersMap.has(grid)).toBe(true);
104+
105+
// Delete from WeakMap
106+
gridWidgetContainersMap.delete(grid);
107+
108+
// Verify it's gone
109+
expect(gridWidgetContainersMap.has(grid)).toBe(false);
110+
});
111+
112+
it('should handle multiple widgets in the same grid', () => {
113+
const grid = new MockGridStack() as any;
114+
const widgets = [
115+
{ id: '1', grid },
116+
{ id: '2', grid },
117+
{ id: '3', grid },
118+
];
119+
const elements = widgets.map(() => document.createElement('div'));
120+
121+
// Simulate renderCB for all widgets
122+
widgets.forEach((widget, index) => {
123+
const element = elements[index];
124+
if (widget.id && widget.grid) {
125+
let containers = gridWidgetContainersMap.get(widget.grid);
126+
if (!containers) {
127+
containers = new Map();
128+
gridWidgetContainersMap.set(widget.grid, containers);
129+
}
130+
containers.set(widget.id, element);
131+
}
132+
});
133+
134+
const containers = gridWidgetContainersMap.get(grid);
135+
expect(containers?.size).toBe(3);
136+
expect(containers?.get('1')).toBe(elements[0]);
137+
expect(containers?.get('2')).toBe(elements[1]);
138+
expect(containers?.get('3')).toBe(elements[2]);
139+
});
140+
});
141+

react/lib/grid-stack-render-provider.tsx

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,9 @@ import { GridStack, GridStackOptions, GridStackWidget } from "gridstack";
1010
import { GridStackRenderContext } from "./grid-stack-render-context";
1111
import isEqual from "react-fast-compare";
1212

13+
// WeakMap to store widget containers for each grid instance
14+
export const gridWidgetContainersMap = new WeakMap<GridStack, Map<string, HTMLElement>>();
15+
1316
export function GridStackRenderProvider({ children }: PropsWithChildren) {
1417
const {
1518
_gridStack: { value: gridStack, set: setGridStack },
@@ -21,8 +24,17 @@ export function GridStackRenderProvider({ children }: PropsWithChildren) {
2124
const optionsRef = useRef<GridStackOptions>(initialOptions);
2225

2326
const renderCBFn = useCallback(
24-
(element: HTMLElement, widget: GridStackWidget) => {
25-
if (widget.id) {
27+
(element: HTMLElement, widget: GridStackWidget & { grid?: GridStack }) => {
28+
if (widget.id && widget.grid) {
29+
// Get or create the widget container map for this grid instance
30+
let containers = gridWidgetContainersMap.get(widget.grid);
31+
if (!containers) {
32+
containers = new Map<string, HTMLElement>();
33+
gridWidgetContainersMap.set(widget.grid, containers);
34+
}
35+
containers.set(widget.id, element);
36+
37+
// Also update the local ref for backward compatibility
2638
widgetContainersRef.current.set(widget.id, element);
2739
}
2840
},
@@ -50,6 +62,8 @@ export function GridStackRenderProvider({ children }: PropsWithChildren) {
5062
gridStack.removeAll(false);
5163
gridStack.destroy(false);
5264
widgetContainersRef.current.clear();
65+
// Clean up the WeakMap entry for this grid instance
66+
gridWidgetContainersMap.delete(gridStack);
5367
optionsRef.current = initialOptions;
5468
setGridStack(initGrid());
5569
} catch (e) {
@@ -73,6 +87,14 @@ export function GridStackRenderProvider({ children }: PropsWithChildren) {
7387
value={useMemo(
7488
() => ({
7589
getWidgetContainer: (widgetId: string) => {
90+
// First try to get from the current grid instance's map
91+
if (gridStack) {
92+
const containers = gridWidgetContainersMap.get(gridStack);
93+
if (containers?.has(widgetId)) {
94+
return containers.get(widgetId) || null;
95+
}
96+
}
97+
// Fallback to local ref for backward compatibility
7698
return widgetContainersRef.current.get(widgetId) || null;
7799
},
78100
}),

react/package.json

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,23 +8,28 @@
88
"start": "vite",
99
"build": "tsc -b && vite build",
1010
"lint": "eslint .",
11-
"preview": "vite preview"
11+
"preview": "vite preview",
12+
"test": "vitest",
13+
"test:ui": "vitest --ui"
1214
},
1315
"dependencies": {
1416
"gridstack": "^12.2.2",
1517
"react": "^18.3.1",
1618
"react-dom": "^18.3.1",
17-
"react-fast-compare": "^3.2.2"
19+
"react-fast-compare": "^3.2.2",
20+
"vitest": "^3.2.4"
1821
},
1922
"devDependencies": {
2023
"@eslint/js": "^9.9.0",
2124
"@types/react": "^18.3.3",
2225
"@types/react-dom": "^18.3.0",
2326
"@vitejs/plugin-react-swc": "^3.5.0",
27+
"@vitest/ui": "^3.2.4",
2428
"eslint": "^9.9.0",
2529
"eslint-plugin-react-hooks": "^5.1.0-rc.0",
2630
"eslint-plugin-react-refresh": "^0.4.9",
2731
"globals": "^15.9.0",
32+
"jsdom": "^26.1.0",
2833
"typescript": "^5.5.3",
2934
"typescript-eslint": "^8.0.1",
3035
"vite": "^5.4.19"

react/src/demo/demo.tsx

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import {
88
GridStackRenderProvider,
99
useGridStackContext,
1010
} from "../../lib";
11+
// import { GridStackRenderProvider } from "../../lib/grid-stack-render-provider-single";
1112

1213
import "gridstack/dist/gridstack.css";
1314
import "./demo.css";
@@ -134,17 +135,26 @@ const gridOptions: GridStackOptions = {
134135
export function GridStackDemo() {
135136
// ! Uncontrolled
136137
const [initialOptions] = useState(gridOptions);
138+
const [initialOptions2] = useState({});
137139

138140
return (
141+
<>
139142
<GridStackProvider initialOptions={initialOptions}>
140143
<Toolbar />
141-
142-
<GridStackRenderProvider>
144+
<GridStackRenderProvider>
143145
<GridStackRender componentMap={COMPONENT_MAP} />
144146
</GridStackRenderProvider>
147+
<DebugInfo />
148+
</GridStackProvider>
145149

150+
<GridStackProvider initialOptions={initialOptions2}>
151+
<Toolbar />
152+
<GridStackRenderProvider>
153+
<GridStackRender componentMap={COMPONENT_MAP} />
154+
</GridStackRenderProvider>
146155
<DebugInfo />
147156
</GridStackProvider>
157+
</>
148158
);
149159
}
150160

react/vite.config.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,4 +4,8 @@ import react from '@vitejs/plugin-react-swc'
44
// https://vitejs.dev/config/
55
export default defineConfig({
66
plugins: [react()],
7+
test: {
8+
environment: 'jsdom',
9+
globals: true,
10+
},
711
})

0 commit comments

Comments
 (0)