Skip to content

Commit f91730f

Browse files
committed
fix: useKeyMap unmount removes keys (3778)
1 parent e7915d8 commit f91730f

File tree

2 files changed

+37
-3
lines changed

2 files changed

+37
-3
lines changed
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
import { renderHook } from '@testing-library/react';
2+
import { useKeyMap } from '../hooks';
3+
import { CodeMirrorEditor, KeyMap } from '../types';
4+
5+
describe('hooks', () => {
6+
describe('useKeyMap', () => {
7+
it('works correctly', () => {
8+
let keys = {};
9+
const editor: Pick<CodeMirrorEditor, 'addKeyMap' | 'removeKeyMap'> = {
10+
addKeyMap: jest.fn((keyMap: KeyMap) => {
11+
keys = { ...keys, ...keyMap };
12+
}),
13+
removeKeyMap: jest.fn(key => {
14+
delete keys[key];
15+
}),
16+
};
17+
const callback = jest.fn();
18+
const { unmount } = renderHook(() =>
19+
useKeyMap(
20+
editor as unknown as CodeMirrorEditor,
21+
['foo', 'bar'],
22+
callback,
23+
),
24+
);
25+
expect(Object.keys(keys).length).toBe(2);
26+
unmount();
27+
expect(Object.keys(keys).length).toBe(0);
28+
});
29+
});
30+
});

packages/graphiql-react/src/editor/hooks.ts

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -136,9 +136,12 @@ export function useKeyMap(
136136
if (!editor) {
137137
return;
138138
}
139-
for (const key of keys) {
140-
editor.removeKeyMap(key);
141-
}
139+
const handleRemoveKeys = () => {
140+
for (const key of keys) {
141+
editor.removeKeyMap(key);
142+
}
143+
};
144+
handleRemoveKeys();
142145

143146
if (callback) {
144147
const keyMap: Record<string, EmptyCallback> = {};
@@ -147,6 +150,7 @@ export function useKeyMap(
147150
}
148151
editor.addKeyMap(keyMap);
149152
}
153+
return handleRemoveKeys;
150154
}, [editor, keys, callback]);
151155
}
152156

0 commit comments

Comments
 (0)