mirror/insomnia
1
0
Fork 0
mirror of https://github.com/Kong/insomnia.git synced 2026-08-03 19:32:31 -04:00
Code Issues Packages Projects Releases Wiki Activity

fix: stop reusing blank-row ids in key-value editors [INS-2598] (#10260)

Browse Source 
* fix: ensure unique blank IDs for environment key-value pairs to prevent stale state

* test: add validation for blank row behavior after deleting committed rows in environment table editor

(cherry picked from commit 4ad32eeb7c)
This commit is contained in:
Pavlos Koutoglou
2026-07-16 18:58:04 +03:00
committed by Insomnia
parent ced564c6c5
commit 2eca27e7dc
3 changed files with 66 additions and 8 deletions
Download Patch File Download Diff File Expand all files Collapse all files

48
packages/insomnia-smoke-test/tests/smoke/key-value-editor-blank-row.test.ts
View File

@@ -85,4 +85,52 @@ test.describe('Key-value editor blank row', () => {
await expect.soft(kvTable.getByRole('option')).toHaveCount(optionsBefore + 1);
await expect.soft(kvTable).toContainText('blankRowKey');
});
test('environment table editor: deleting a committed row does not leave stale text on the blank row', async ({ page, app }) => {
const text = await loadFixture('environments.yaml');
await app.evaluate(async ({ clipboard }, text) => clipboard.writeText(text), text);
await page.getByLabel('Import').click();
await page.locator('[data-test-id="import-from-clipboard"]').click();
await page.getByRole('button', { name: 'Scan' }).click();
await page.getByRole('dialog').getByRole('button', { name: 'Import' }).click();
await page.getByRole('dialog').waitFor({ state: 'hidden' });
await page.getByRole('button', { name: 'Manage Environments' }).click();
await page.getByRole('button', { name: 'Manage collection environments' }).click();
await page.getByLabel('Environments', { exact: true }).getByText('ExampleA').click();
await page.getByRole('button', { name: 'Table Edit' }).click();
const kvTable = page.getByRole('listbox', { name: 'Environment Key Value Pair' });
await expect.soft(kvTable).toContainText('exampleString');
const optionsBefore = await kvTable.getByRole('option').count();
// Commit two new pairs, one after another, via the trailing blank row. Each commit is
// awaited (a fresh blank row appears) before typing into the next one, so the second
// keystroke does not land in the still-committing first row.
await kvTable.getByRole('option').last().getByTestId('OneLineEditor').first().locator('.CodeMirror').click();
await page.keyboard.type('firstNewKey');
await expect.soft(kvTable.getByRole('option')).toHaveCount(optionsBefore + 1);
await kvTable.getByRole('option').last().getByTestId('OneLineEditor').first().locator('.CodeMirror').click();
await page.keyboard.type('secondNewKey');
await expect.soft(kvTable.getByRole('option')).toHaveCount(optionsBefore + 2);
// Delete the first of the two new rows (a PromptButton: first click arms it, second confirms).
const firstRow = kvTable.getByRole('option').filter({ hasText: 'firstNewKey' });
const deleteButton = firstRow.getByRole('button', { name: 'Delete Row' });
await deleteButton.click();
await deleteButton.click();
// The deleted row is gone, the other committed row remains, and the trailing blank row
// is still present and empty (it must not resurrect the deleted row's text).
await expect.soft(kvTable).not.toContainText('firstNewKey');
await expect.soft(kvTable).toContainText('secondNewKey');
// Typing into the blank row must produce only the new text, confirming its id/editor
// instance was not silently reused from the deleted row.
await kvTable.getByRole('option').last().getByTestId('OneLineEditor').first().locator('.CodeMirror').click();
await page.keyboard.type('thirdNewKey');
await expect.soft(kvTable).not.toContainText('firstNewKey');
await expect.soft(kvTable).toContainText('thirdNewKey');
});
});

13
packages/insomnia/src/ui/components/editors/environment-key-value-editor/key-value-editor.tsx
View File

@@ -81,12 +81,17 @@ export const EnvironmentKVEditor = ({
// held in state) so it only changes when the data actually changes. This keeps it in
// sync with the async data updates - if it flipped eagerly the row the user just typed
// into would briefly belong to no list and flicker.
// A fresh id is minted only once the previous one is committed (found in persistedPairs) -
// never reused, even after that row is later deleted. Reclaiming a freed numeric slot would
// let a deleted row's id land back on the blank row, and since the row's key (and its
// OneLineEditor's key) is derived from that id, React would reuse the deleted row's DOM/editor
// instance instead of remounting it - leaving stale, uncommitted text on screen.
const blankIdRef = useRef(generateId('envPair-blank'));
const blankId = useMemo(() => {
let n = 0;
while (persistedPairs.some(p => p.id === `envPair-blank-${n}`)) {
n++;
if (persistedPairs.some(p => p.id === blankIdRef.current)) {
blankIdRef.current = generateId('envPair-blank');
}
return `envPair-blank-${n}`;
return blankIdRef.current;
}, [persistedPairs]);
const blankPair: EnvironmentKvPairData = useMemo(
() => ({ id: blankId, name: '', value: '', type: EnvironmentKvPairDataType.STRING, enabled: true }),

13
packages/insomnia/src/ui/components/key-value-editor/key-value-editor.tsx
View File

@@ -109,12 +109,17 @@ export const KeyValueEditor: FC<Props> = ({
// held in state) so it only changes when the data actually changes. This keeps it in
// sync with the async data updates - if it flipped eagerly the row the user just typed
// into would briefly belong to no list and flicker.
// A fresh id is minted only once the previous one is committed (found in persistedItems) -
// never reused, even after that row is later deleted. Reclaiming a freed numeric slot would
// let a deleted row's id land back on the blank row, and since the row's key (and its
// OneLineEditor's key) is derived from that id, React would reuse the deleted row's DOM/editor
// instance instead of remounting it - leaving stale, uncommitted text on screen.
const blankIdRef = useRef(generateId('pair-blank'));
const blankId = useMemo(() => {
let n = 0;
while (persistedItems.some(item => item.id === `pair-blank-${n}`)) {
n++;
if (persistedItems.some(item => item.id === blankIdRef.current)) {
blankIdRef.current = generateId('pair-blank');
}
return `pair-blank-${n}`;
return blankIdRef.current;
}, [persistedItems]);
const blankPair = useMemo<Pair>(
() => ({ id: blankId, name: '', value: '', description: '', disabled: false }),