Guard focus() against an uninitialized editor - #1177
Conversation
`focus()` reads `this.editor`, which is only assigned in connectedCallback. When `focus()` runs before the editor is initialized, the `#isContentFocused` guard evaluates to false and execution falls through to `this.editor.focus()`, throwing "undefined is not an object (evaluating 'this.editor.focus')". This can happen when using Lexxy alongside Turbo and autofocus. Turbo can call `focus()` on the element before the editor is initialized, leading to an error. This commit makes Lexxy return early when `this.editor` is not set. Autofocus still works through the element's own `#handleAutofocus`, which runs after the editor is created.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
Prevents lexxy-editor’s focus() override from throwing when invoked before connectedCallback() initializes this.editor (e.g., Turbo + autofocus calling focus() early). Adds a Playwright regression test ensuring early focus() is a safe no-op while preserving the existing autofocus behavior via #handleAutofocus() after initialization.
Tip
If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or run gh pr ready --undo.
Click "Ready for review" or run gh pr ready to reengage.
Changes:
- Guard
focus()withif (!this.editor || this.#isContentFocused) returnto avoid calling.focus()on an uninitialized editor instance. - Add a browser test asserting
editor.focus()on a newly-created (unconnected)<lexxy-editor>does not throw.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
src/elements/editor.js |
Adds an early return in focus() when this.editor hasn’t been created yet. |
test/browser/tests/editor/focus.test.js |
Adds a regression test verifying focus() is a safe no-op before initialization. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
samuelpecher
left a comment
samuelpecher
left a comment
There was a problem hiding this comment.
What's the tradeoff in making this a no-op vs storing the focus intent and focusing the editor on eventual load?
I think there's no need to do that for my use-case, at least. Lexxy's own |
Co-authored-by: Samuel Péchèr <samp@hey.com>
focus()readsthis.editor, which is only assigned in connectedCallback. Whenfocus()runs before the editor is initialized, the#isContentFocusedguard evaluates to false and execution falls through tothis.editor.focus(), throwing "undefined is not an object (evaluating 'this.editor.focus')".This can happen when using Lexxy alongside Turbo and autofocus. Turbo can call
focus()on the element before the editor is initialized, leading to an error.This commit makes Lexxy return early when
this.editoris not set. Autofocus still works through the element's own#handleAutofocus, which runs after the editor is created.