Skip to content

Commit 70cde1d

Browse files
e111077e111077
committed
fix(editor): only fire change event on non-programmatic changes
1 parent b2af5c8 commit 70cde1d

File tree

2 files changed

+137
-10
lines changed

2 files changed

+137
-10
lines changed

‎src/playground-code-editor.ts‎

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -339,7 +339,8 @@ export class PlaygroundCodeEditor extends LitElement {
339339
@queryAssignedElements({slot: 'extensions', flatten: true})
340340
private _extensionElements!: HTMLElement[];
341341

342-
private _valueChangingFromOutside = false;
342+
// Annotation to mark programmatic changes (not user edits)
343+
private readonly _programmaticChange = Annotation.define<boolean>();
343344
private _diagnosticDecorations: DecorationSet = Decoration.none;
344345
private _diagnosticsMouseoverListenerActive = false;
345346
private _lastTransactions: Transaction[] = [];
@@ -424,6 +425,7 @@ export class PlaygroundCodeEditor extends LitElement {
424425
insert: this.value ?? '',
425426
},
426427
],
428+
annotations: this._programmaticChange.of(true),
427429
});
428430
}
429431
}
@@ -447,14 +449,13 @@ export class PlaygroundCodeEditor extends LitElement {
447449
insert: this.value ?? '',
448450
},
449451
],
452+
annotations: this._programmaticChange.of(true),
450453
});
451454
docState = tempView.state;
452455
this._docCache.set(docKey, docState);
453456
tempView.destroy();
454457
}
455458

456-
this._valueChangingFromOutside = true;
457-
458459
// Replace the entire view with the new editor state. Unlike CM5, CM6
459460
// is modular, and the history stays on the state object rather than
460461
// the editor / view.
@@ -465,8 +466,6 @@ export class PlaygroundCodeEditor extends LitElement {
465466
if (needsHideAndFold) {
466467
void this._applyHideAndFoldRegions();
467468
}
468-
469-
this._valueChangingFromOutside = false;
470469
break;
471470
}
472471
case 'value':
@@ -476,8 +475,6 @@ export class PlaygroundCodeEditor extends LitElement {
476475
}
477476

478477
if (this.value !== view?.state.doc.toString()) {
479-
this._valueChangingFromOutside = true;
480-
481478
// The 'value' property was changed externally and it differs from
482479
// the editor's current document content, so we need to update the
483480
// view model to match.
@@ -492,9 +489,8 @@ export class PlaygroundCodeEditor extends LitElement {
492489
insert: this.value ?? '',
493490
},
494491
],
492+
annotations: this._programmaticChange.of(true),
495493
});
496-
497-
this._valueChangingFromOutside = false;
498494
}
499495
break;
500496
case 'lineNumbers':
@@ -720,9 +716,14 @@ export class PlaygroundCodeEditor extends LitElement {
720716
tr.annotation(Transaction.userEvent) === 'redo'
721717
);
722718

719+
// Check if this change was programmatic (not a user edit)
720+
const isProgrammatic = update.transactions.some(
721+
(tr) => tr.annotation(this._programmaticChange) === true
722+
);
723+
723724
// External changes are usually things like the editor switching which
724725
// file it is displaying.
725-
if (this._valueChangingFromOutside) {
726+
if (isProgrammatic) {
726727
// Apply hide/fold regions when value changes from outside
727728
void this._applyHideAndFoldRegions();
728729
this._showDiagnostics();
@@ -731,6 +732,7 @@ export class PlaygroundCodeEditor extends LitElement {
731732
// Always reapply hide/fold regions after undo/redo
732733
void this._applyHideAndFoldRegions();
733734
}
735+
// Only fire change event for user-initiated edits
734736
this.dispatchEvent(new Event('change'));
735737
}
736738
}

‎src/test/playground-code-editor_test.ts‎

Lines changed: 125 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,131 @@ suite('playground-code-editor', () => {
7373
});
7474
});
7575

76+
test('does NOT dispatch change event when switching documentKey and value together', async () => {
77+
const editor = document.createElement('playground-code-editor');
78+
const DOCUMENT_KEY1 = { doc: 1 };
79+
const DOCUMENT_KEY2 = { doc: 2 };
80+
81+
editor.value = 'document 1';
82+
editor.documentKey = DOCUMENT_KEY1;
83+
container.appendChild(editor);
84+
await editor.updateComplete;
85+
await raf();
86+
87+
let changeCount = 0;
88+
editor.addEventListener('change', () => {
89+
changeCount++;
90+
});
91+
92+
93+
// This simulates what playground-file-editor does when switching files
94+
// Both value AND documentKey change together in a single update
95+
editor.value = 'document 2';
96+
editor.documentKey = DOCUMENT_KEY2;
97+
98+
// Wait for Lit's update cycle
99+
await editor.updateComplete;
100+
await raf();
101+
102+
// Wait for any potential async change events
103+
await wait(100);
104+
105+
// Switch back - this is where the bug manifests in lit.dev
106+
editor.value = 'document 1 modified';
107+
editor.documentKey = DOCUMENT_KEY1;
108+
await editor.updateComplete;
109+
await raf();
110+
await wait(100);
111+
112+
113+
assert.equal(
114+
changeCount,
115+
0,
116+
'Switching documentKey and value together should not fire change event'
117+
);
118+
});
119+
120+
test('does NOT dispatch change event when switching documentKey', async () => {
121+
const editor = document.createElement('playground-code-editor');
122+
const DOCUMENT_KEY1 = { dockey: 1 };
123+
const DOCUMENT_KEY2 = { dockey: 2 };
124+
125+
editor.value = 'document 1';
126+
editor.documentKey = DOCUMENT_KEY1;
127+
container.appendChild(editor);
128+
await editor.updateComplete;
129+
await raf();
130+
131+
let changeCount = 0;
132+
editor.addEventListener('change', () => changeCount++);
133+
134+
// Switch to a different document - should NOT fire change event
135+
editor.value = 'document 2';
136+
editor.documentKey = DOCUMENT_KEY2;
137+
await editor.updateComplete;
138+
await raf();
139+
140+
assert.equal(
141+
changeCount,
142+
0,
143+
'Switching documentKey should not fire change event'
144+
);
145+
146+
// Switch back to first document - should NOT fire change event
147+
editor.value = 'document 1';
148+
editor.documentKey = DOCUMENT_KEY1;
149+
await editor.updateComplete;
150+
await raf();
151+
152+
assert.equal(
153+
changeCount,
154+
0,
155+
'Switching back to original documentKey should not fire change event'
156+
);
157+
});
158+
159+
test('dispatches change event only for user edits, not programmatic changes', async () => {
160+
const editor = document.createElement('playground-code-editor');
161+
editor.value = 'foo';
162+
container.appendChild(editor);
163+
await editor.updateComplete;
164+
await raf();
165+
166+
const editorInternals = editor as unknown as {
167+
_editorView: EditorView;
168+
};
169+
170+
let changeCount = 0;
171+
editor.addEventListener('change', () => changeCount++);
172+
173+
// Programmatic change - should NOT fire
174+
editor.value = 'bar';
175+
await editor.updateComplete;
176+
await raf();
177+
assert.equal(changeCount, 0, 'No change event after programmatic update');
178+
179+
// User edit via CodeMirror - SHOULD fire
180+
editorInternals._editorView.dispatch({
181+
changes: {
182+
from: 0,
183+
to: editorInternals._editorView.state.doc.length,
184+
insert: 'baz',
185+
},
186+
});
187+
await raf();
188+
assert.equal(changeCount, 1, 'Change event fired after user edit');
189+
190+
// Another programmatic change - should NOT fire
191+
editor.value = 'qux';
192+
await editor.updateComplete;
193+
await raf();
194+
assert.equal(
195+
changeCount,
196+
1,
197+
'No additional change event after another programmatic update'
198+
);
199+
});
200+
76201
suite('history', () => {
77202
let editor: PlaygroundCodeEditor;
78203
let editorInternals: {

0 commit comments

Comments
 (0)