Skip to content

Commit e85767a

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

File tree

3 files changed

+141
-13
lines changed

3 files changed

+141
-13
lines changed

README.md

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -861,9 +861,9 @@ A pure text editor based on CodeMirror with syntax highlighting for HTML, CSS, J
861861

862862
### Events
863863

864-
| Event | Description |
865-
| -------- | ------------------------------------ |
866-
| `change` | User made an edit to the active file |
864+
| Event | Description |
865+
| -------- | -------------------------------------------------------------------------------------------------------------------------------------------------- |
866+
| `change` | User made an edit to the active file (note: this event is not fired for programmatic changes to the value property nor for the user changing tabs) |
867867

868868
### Keyboard shortcuts
869869

src/playground-code-editor.ts

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,9 @@ interface TypedMap<T> extends Map<keyof T, unknown> {
9191

9292
const unreachable = (n: never) => n;
9393

94+
// Annotation to mark programmatic changes (not user edits)
95+
const programmaticChangeAnnotation = Annotation.define<boolean>();
96+
9497
/**
9598
* A basic text editor with syntax highlighting for HTML, CSS, and JavaScript.
9699
*/
@@ -339,7 +342,6 @@ export class PlaygroundCodeEditor extends LitElement {
339342
@queryAssignedElements({slot: 'extensions', flatten: true})
340343
private _extensionElements!: HTMLElement[];
341344

342-
private _valueChangingFromOutside = false;
343345
private _diagnosticDecorations: DecorationSet = Decoration.none;
344346
private _diagnosticsMouseoverListenerActive = false;
345347
private _lastTransactions: Transaction[] = [];
@@ -424,6 +426,7 @@ export class PlaygroundCodeEditor extends LitElement {
424426
insert: this.value ?? '',
425427
},
426428
],
429+
annotations: programmaticChangeAnnotation.of(true),
427430
});
428431
}
429432
}
@@ -447,14 +450,13 @@ export class PlaygroundCodeEditor extends LitElement {
447450
insert: this.value ?? '',
448451
},
449452
],
453+
annotations: programmaticChangeAnnotation.of(true),
450454
});
451455
docState = tempView.state;
452456
this._docCache.set(docKey, docState);
453457
tempView.destroy();
454458
}
455459

456-
this._valueChangingFromOutside = true;
457-
458460
// Replace the entire view with the new editor state. Unlike CM5, CM6
459461
// is modular, and the history stays on the state object rather than
460462
// the editor / view.
@@ -465,8 +467,6 @@ export class PlaygroundCodeEditor extends LitElement {
465467
if (needsHideAndFold) {
466468
void this._applyHideAndFoldRegions();
467469
}
468-
469-
this._valueChangingFromOutside = false;
470470
break;
471471
}
472472
case 'value':
@@ -476,8 +476,6 @@ export class PlaygroundCodeEditor extends LitElement {
476476
}
477477

478478
if (this.value !== view?.state.doc.toString()) {
479-
this._valueChangingFromOutside = true;
480-
481479
// The 'value' property was changed externally and it differs from
482480
// the editor's current document content, so we need to update the
483481
// view model to match.
@@ -492,9 +490,8 @@ export class PlaygroundCodeEditor extends LitElement {
492490
insert: this.value ?? '',
493491
},
494492
],
493+
annotations: programmaticChangeAnnotation.of(true),
495494
});
496-
497-
this._valueChangingFromOutside = false;
498495
}
499496
break;
500497
case 'lineNumbers':
@@ -720,9 +717,16 @@ export class PlaygroundCodeEditor extends LitElement {
720717
tr.annotation(Transaction.userEvent) === 'redo'
721718
);
722719

720+
// Check if ALL transactions are programmatic (not user edits).
721+
// We fire the change event if ANY transaction is a user edit, even if
722+
// there are also programmatic transactions in the same update.
723+
const allProgrammatic = update.transactions.every(
724+
(tr) => tr.annotation(programmaticChangeAnnotation) === true
725+
);
726+
723727
// External changes are usually things like the editor switching which
724728
// file it is displaying.
725-
if (this._valueChangingFromOutside) {
729+
if (allProgrammatic) {
726730
// Apply hide/fold regions when value changes from outside
727731
void this._applyHideAndFoldRegions();
728732
this._showDiagnostics();
@@ -731,6 +735,7 @@ export class PlaygroundCodeEditor extends LitElement {
731735
// Always reapply hide/fold regions after undo/redo
732736
void this._applyHideAndFoldRegions();
733737
}
738+
// Only fire change event for user-initiated edits
734739
this.dispatchEvent(new Event('change'));
735740
}
736741
}

src/test/playground-code-editor_test.ts

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

0 commit comments

Comments
 (0)