diff --git a/README.md b/README.md index 40c39bc1..a2db26ad 100644 --- a/README.md +++ b/README.md @@ -861,9 +861,9 @@ A pure text editor based on CodeMirror with syntax highlighting for HTML, CSS, J ### Events -| Event | Description | -| -------- | ------------------------------------ | -| `change` | User made an edit to the active file | +| Event | Description | +| -------- | -------------------------------------------------------------------------------------------------------------------------------------------------- | +| `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) | ### Keyboard shortcuts diff --git a/src/playground-code-editor.ts b/src/playground-code-editor.ts index 4d03413a..829b8120 100644 --- a/src/playground-code-editor.ts +++ b/src/playground-code-editor.ts @@ -91,6 +91,9 @@ interface TypedMap extends Map { const unreachable = (n: never) => n; +// Annotation to mark programmatic changes (not user edits) +const programmaticChangeAnnotation = Annotation.define(); + /** * A basic text editor with syntax highlighting for HTML, CSS, and JavaScript. */ @@ -339,7 +342,6 @@ export class PlaygroundCodeEditor extends LitElement { @queryAssignedElements({slot: 'extensions', flatten: true}) private _extensionElements!: HTMLElement[]; - private _valueChangingFromOutside = false; private _diagnosticDecorations: DecorationSet = Decoration.none; private _diagnosticsMouseoverListenerActive = false; private _lastTransactions: Transaction[] = []; @@ -424,6 +426,7 @@ export class PlaygroundCodeEditor extends LitElement { insert: this.value ?? '', }, ], + annotations: programmaticChangeAnnotation.of(true), }); } } @@ -447,14 +450,13 @@ export class PlaygroundCodeEditor extends LitElement { insert: this.value ?? '', }, ], + annotations: programmaticChangeAnnotation.of(true), }); docState = tempView.state; this._docCache.set(docKey, docState); tempView.destroy(); } - this._valueChangingFromOutside = true; - // Replace the entire view with the new editor state. Unlike CM5, CM6 // is modular, and the history stays on the state object rather than // the editor / view. @@ -465,8 +467,6 @@ export class PlaygroundCodeEditor extends LitElement { if (needsHideAndFold) { void this._applyHideAndFoldRegions(); } - - this._valueChangingFromOutside = false; break; } case 'value': @@ -476,8 +476,6 @@ export class PlaygroundCodeEditor extends LitElement { } if (this.value !== view?.state.doc.toString()) { - this._valueChangingFromOutside = true; - // The 'value' property was changed externally and it differs from // the editor's current document content, so we need to update the // view model to match. @@ -492,9 +490,8 @@ export class PlaygroundCodeEditor extends LitElement { insert: this.value ?? '', }, ], + annotations: programmaticChangeAnnotation.of(true), }); - - this._valueChangingFromOutside = false; } break; case 'lineNumbers': @@ -720,9 +717,16 @@ export class PlaygroundCodeEditor extends LitElement { tr.annotation(Transaction.userEvent) === 'redo' ); + // Check if ALL transactions are programmatic (not user edits). + // We fire the change event if ANY transaction is a user edit, even if + // there are also programmatic transactions in the same update. + const allProgrammatic = update.transactions.every( + (tr) => tr.annotation(programmaticChangeAnnotation) === true + ); + // External changes are usually things like the editor switching which // file it is displaying. - if (this._valueChangingFromOutside) { + if (allProgrammatic) { // Apply hide/fold regions when value changes from outside void this._applyHideAndFoldRegions(); this._showDiagnostics(); @@ -731,6 +735,7 @@ export class PlaygroundCodeEditor extends LitElement { // Always reapply hide/fold regions after undo/redo void this._applyHideAndFoldRegions(); } + // Only fire change event for user-initiated edits this.dispatchEvent(new Event('change')); } } diff --git a/src/test/playground-code-editor_test.ts b/src/test/playground-code-editor_test.ts index 8eb61f4b..93b1078e 100644 --- a/src/test/playground-code-editor_test.ts +++ b/src/test/playground-code-editor_test.ts @@ -73,6 +73,129 @@ suite('playground-code-editor', () => { }); }); + test('does NOT dispatch change event when switching documentKey and value together', async () => { + const editor = document.createElement('playground-code-editor'); + const DOCUMENT_KEY1 = {doc: 1}; + const DOCUMENT_KEY2 = {doc: 2}; + + editor.value = 'document 1'; + editor.documentKey = DOCUMENT_KEY1; + container.appendChild(editor); + await editor.updateComplete; + await raf(); + + let changeCount = 0; + editor.addEventListener('change', () => { + changeCount++; + }); + + // This simulates what playground-file-editor does when switching files + // Both value AND documentKey change together in a single update + editor.value = 'document 2'; + editor.documentKey = DOCUMENT_KEY2; + + // Wait for Lit's update cycle + await editor.updateComplete; + await raf(); + + // Wait for any potential async change events + await wait(100); + + // Switch back - this is where the bug manifests in lit.dev + editor.value = 'document 1 modified'; + editor.documentKey = DOCUMENT_KEY1; + await editor.updateComplete; + await raf(); + await wait(100); + + assert.equal( + changeCount, + 0, + 'Switching documentKey and value together should not fire change event' + ); + }); + + test('does NOT dispatch change event when switching documentKey', async () => { + const editor = document.createElement('playground-code-editor'); + const DOCUMENT_KEY1 = {dockey: 1}; + const DOCUMENT_KEY2 = {dockey: 2}; + + editor.value = 'document 1'; + editor.documentKey = DOCUMENT_KEY1; + container.appendChild(editor); + await editor.updateComplete; + await raf(); + + let changeCount = 0; + editor.addEventListener('change', () => changeCount++); + + // Switch to a different document - should NOT fire change event + editor.value = 'document 2'; + editor.documentKey = DOCUMENT_KEY2; + await editor.updateComplete; + await raf(); + + assert.equal( + changeCount, + 0, + 'Switching documentKey should not fire change event' + ); + + // Switch back to first document - should NOT fire change event + editor.value = 'document 1'; + editor.documentKey = DOCUMENT_KEY1; + await editor.updateComplete; + await raf(); + + assert.equal( + changeCount, + 0, + 'Switching back to original documentKey should not fire change event' + ); + }); + + test('dispatches change event only for user edits, not programmatic changes', async () => { + const editor = document.createElement('playground-code-editor'); + editor.value = 'foo'; + container.appendChild(editor); + await editor.updateComplete; + await raf(); + + const editorInternals = editor as unknown as { + _editorView: EditorView; + }; + + let changeCount = 0; + editor.addEventListener('change', () => changeCount++); + + // Programmatic change - should NOT fire + editor.value = 'bar'; + await editor.updateComplete; + await raf(); + assert.equal(changeCount, 0, 'No change event after programmatic update'); + + // User edit via CodeMirror - SHOULD fire + editorInternals._editorView.dispatch({ + changes: { + from: 0, + to: editorInternals._editorView.state.doc.length, + insert: 'baz', + }, + }); + await raf(); + assert.equal(changeCount, 1, 'Change event fired after user edit'); + + // Another programmatic change - should NOT fire + editor.value = 'qux'; + await editor.updateComplete; + await raf(); + assert.equal( + changeCount, + 1, + 'No additional change event after another programmatic update' + ); + }); + suite('history', () => { let editor: PlaygroundCodeEditor; let editorInternals: {