Conversation
WalkthroughLarge repository-wide migration to TypeScript and stronger typing across packages/koenig-lexical and demo. Replaced many runtime PropTypes/JS modules with typed .ts/.tsx counterparts, added numerous exported interfaces and component/hook/node/plugin/context prop types, tightened event/ref/nullability handling, introduced new components/modules (e.g., CodeBlockCard, UnsplashModal, TransistorCard, TextInput), removed some legacy JS modules, updated Storybook stories with // Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
kevinansfield
force-pushed
the
ts/koenig-lexical-cleanup
branch
from
d5269fd to
9d91044
Compare
kevinansfield
changed the title
kevinansfield
force-pushed
the
ts/koenig-lexical-cleanup
branch
3 times, most recently
from
5516184 to
c91da8a
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (23)
packages/koenig-lexical/demo/components/Navigator.tsx (1)
9-16:⚠️ Potential issue | 🟠 MajorWrap the side effect in
useEffectand add cleanup.The global window mutation on line 13 runs in the component body, which executes on every render. React side effects should be placed in
useEffectto:
- Run only when dependencies change (mount/navigate updates)
- Enable proper cleanup on unmount to avoid stale references
🔧 Proposed fix using useEffect with cleanup
+import {useEffect} from 'react'; import {useNavigate} from 'react-router-dom'; declare global { interface Window { navigate: ReturnType<typeof useNavigate>; } } const Navigator = () => { const navigate = useNavigate(); - // Hack, used to allow Playwright to navigate without triggering a full page reload. - window.navigate = navigate; + useEffect(() => { + // Hack, used to allow Playwright to navigate without triggering a full page reload. + window.navigate = navigate; + + return () => { + delete window.navigate; + }; + }, [navigate]); return null; }; export default Navigator;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/koenig-lexical/demo/components/Navigator.tsx` around lines 9 - 16, The Navigator component assigns window.navigate in the render body which runs every render; move that side effect into a useEffect inside the Navigator function so the assignment runs only when the navigate function changes (use [navigate] as dependency), perform a safety check for window (typeof window !== 'undefined') and add cleanup in the effect to remove the global (either delete window.navigate or set it to undefined) on unmount to avoid stale references.packages/koenig-lexical/src/hooks/usePinturaEditor.ts (1)
166-169:⚠️ Potential issue | 🔴 CriticalEvent listener not properly removed due to mismatched
captureoption.The listener is added with
{capture: true}at line 166, but removed without it at line 169. Thecaptureflag must match forremoveEventListenerto find and remove the correct listener. This causes a memory leak where handlers accumulate on each component remount.🐛 Proposed fix
return () => { - window.removeEventListener('click', handleCloseClick); + window.removeEventListener('click', handleCloseClick, {capture: true}); };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/koenig-lexical/src/hooks/usePinturaEditor.ts` around lines 166 - 169, The event listener added in usePinturaEditor uses window.addEventListener('click', handleCloseClick, {capture: true}) but is removed without the same options, so removeEventListener won't match; update the cleanup in the usePinturaEditor hook to call window.removeEventListener('click', handleCloseClick, {capture: true}) (matching the addEventListener options) so the handleCloseClick listener is correctly removed on unmount/remount.packages/koenig-lexical/src/components/ui/TenorSelector.tsx (1)
74-80:⚠️ Potential issue | 🟠 Major
handleGifHighlightreferenced before definition causes stale closure.The
useEffecton lines 74-80 referenceshandleGifHighlightin its dependency array, buthandleGifHighlightis defined later (line 225) and recreated on every render. This can cause the event listener to reference stale state.Wrap handleGifHighlight in useCallback or move before the effect
+ const handleGifHighlight = React.useCallback((event: KeyboardEvent) => { + switch (event.key) { + case 'Tab': + return handleTab(event); + // ... rest of cases + } + }, [highlightedGif, gifs, columns]); + useEffect(() => { document.addEventListener('keydown', handleGifHighlight); return () => { document.removeEventListener('keydown', handleGifHighlight); }; }, [handleGifHighlight]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/koenig-lexical/src/components/ui/TenorSelector.tsx` around lines 74 - 80, The effect that adds/removes the keydown listener references handleGifHighlight which is defined later and recreated each render; make handleGifHighlight stable by wrapping it in useCallback (e.g., useCallback(handleGifHighlight, [/* include any state/props it uses */])) or move its definition above the useEffect so the same function instance is used for add/remove, and then ensure the useEffect dependency array contains the stable handleGifHighlight (or the correct dependency list) so the cleanup/unsubscribe works with the exact same function reference.packages/koenig-lexical/src/hooks/useTypeaheadTriggerMatch.ts (1)
15-21:⚠️ Potential issue | 🟠 MajorEscape
triggerbefore interpolating into regex character classes.
Lines 15 and 17 concatenatetriggerdirectly into regex character classes without escaping. Special characters like],^,\,-change regex semantics or create invalid patterns. For example, a trigger value of]would produce invalid syntax, and^would negate the character class instead of matching the literal character.Proposed patch
+const escapeForCharClass = (value: string) => value.replace(/[\\\]\[-^]/g, '\\$&'); + export default function useBasicTypeaheadTriggerMatch(trigger: string, {minLength = 1, maxLength = 75}: {minLength?: number; maxLength?: number}) { return useCallback( (text: string): TriggerMatch | null => { - const invalidChars = '[^' + trigger + '\\s]'; // escaped set - these cannot be present in the matched string + const safeTrigger = escapeForCharClass(trigger); + const invalidChars = `[^${safeTrigger}\\s]`; // escaped set - these cannot be present in the matched string const TypeaheadTriggerRegex = new RegExp( - '[' + trigger + ']' + + '[' + safeTrigger + ']' + '(' + '(?:' + invalidChars + ')' + '{0,' + maxLength + '}' + ')$', );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/koenig-lexical/src/hooks/useTypeaheadTriggerMatch.ts` around lines 15 - 21, The code builds character classes using the raw trigger string (see invalidChars and TypeaheadTriggerRegex) which breaks for metacharacters like ], ^, -, \; fix by escaping regex-special characters in trigger before interpolating it into the character-class expressions (create an escapeRegExp helper or reuse one, produce escapedTrigger = escapeRegExp(trigger)), then use escapedTrigger when constructing invalidChars and the TypeaheadTriggerRegex so the trigger is always treated as a literal character in the regex.packages/koenig-lexical/src/components/ui/cards/MarkdownCard/MarkdownEditor.tsx (1)
163-175:⚠️ Potential issue | 🟡 MinorNon-null assertions throughout assume editor is always initialized.
Functions like
addShortcuts,toggleSpellcheck,toggleButtonClass,closeHelpDialog, andcloseUnsplashDialogall usemarkdownEditor.current!. If any of these are called after unmount (e.g., during async cleanup or event handler race conditions), they will throw.Consider adding a guard at the top of each function, or creating a helper that safely accesses the editor:
🛡️ Example helper approach
function getEditor(): SimpleMDEInstance | null { return markdownEditor.current; } function addShortcuts() { const editor = getEditor(); if (!editor) return; const codemirror = editor.codemirror; // ... }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/koenig-lexical/src/components/ui/cards/MarkdownCard/MarkdownEditor.tsx` around lines 163 - 175, Several functions (addShortcuts, toggleSpellcheck, toggleButtonClass, closeHelpDialog, closeUnsplashDialog) directly use markdownEditor.current! and can throw if the editor is unmounted; create a small safe accessor (e.g., getEditor() that returns markdownEditor.current or null) and at the top of each affected function use const editor = getEditor(); if (!editor) return; then use editor.codemirror instead of markdownEditor.current! and update calls like codemirror.setOption('extraKeys', ...) to operate only when editor is present so no non-null assertions remain.packages/koenig-lexical/src/components/ui/cards/MarkdownCard/useMarkdownImageUploader.ts (1)
53-88:⚠️ Potential issue | 🟡 MinorNon-null assertions on
editor.currentmay cause runtime errors.Lines 54 and 83 use
editor.current!to assert the editor is non-null, butSimpleMDEEditorRefexplicitly typescurrentas{ ... } | null. IfinsertImagesis called before the editor initializes or after it unmounts, this will throw a runtime exception.Consider adding an early return guard or using optional chaining with appropriate fallback behavior:
🛡️ Proposed guard
function insertImages(urls: (string | UploadResult)[] = []) { + if (!editor.current) { + return; + } - const codemirror = editor.current!.codemirror; + const codemirror = editor.current.codemirror; // ... rest of function - editor.current!.codemirror.focus(); + editor.current.codemirror.focus();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/koenig-lexical/src/components/ui/cards/MarkdownCard/useMarkdownImageUploader.ts` around lines 53 - 88, The insertImages function uses non-null assertions on editor.current (SimpleMDEEditorRef) and then calls editor.current.codemirror.focus() and codemirror.replaceSelection(...), which can throw if the ref is null; update insertImages to early-return when editor.current is null (e.g., const ed = editor.current; if (!ed) return) and then use the local ed.codemirror safely (or optional chaining) before calling focus() and replaceSelection to avoid runtime exceptions when the editor is not initialized or has unmounted.packages/koenig-lexical/src/components/ui/Modal.tsx (1)
12-17:⚠️ Potential issue | 🟠 MajorMove
preventDefault()andstopPropagation()inside the Escape key conditionCalling
preventDefault()on all keydown events (lines 13–14) blocks keyboard interaction with any interactive content inside the modal. Restrict these calls to Escape key handling only.Proposed fix
const controlByKeys = (event: React.KeyboardEvent) => { - event.stopPropagation(); - event.preventDefault(); - if (event.key === 'Escape') { + event.stopPropagation(); + event.preventDefault(); onClose(); } };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/koenig-lexical/src/components/ui/Modal.tsx` around lines 12 - 17, In controlByKeys, stop calling event.preventDefault() and event.stopPropagation() for every key; instead, detect if event.key === 'Escape' and then call event.preventDefault() and event.stopPropagation() immediately before invoking onClose(); leave all other keys untouched so interactive content inside the modal can receive normal keyboard events. Ensure the change is made inside the controlByKeys function and keeps the onClose() call tied to the Escape branch.packages/koenig-lexical/src/components/ui/Button.tsx (1)
49-49:⚠️ Potential issue | 🟠 MajorLogic bug:
width === 'full'never appliesw-fullclass.The expression
width === 'regular' || 'w-full'is always truthy:
- When
width === 'regular': evaluates totrue- When
width === 'full': evaluates to'w-full'(truthy string)This means the
w-fullclass is always included, regardless of thewidthprop value. The intended logic appears to be applyingw-fullonly whenwidth === 'full'.Proposed fix
className={clsx( 'not-kg-prose inline-block cursor-pointer text-center font-sans font-medium', (!shrink && 'shrink-0'), - width === 'regular' || 'w-full', + width === 'full' && 'w-full', rounded && 'rounded-md',🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/koenig-lexical/src/components/ui/Button.tsx` at line 49, The class expression in Button.tsx mistakenly uses "width === 'regular' || 'w-full'", which always evaluates truthy and thus always adds "w-full"; update the conditional so "w-full" is only added when the width prop equals 'full' (e.g., replace that part with a conditional that checks width === 'full' before adding 'w-full'). Locate the Button component and the className/array where the width variable is used and change the faulty expression to a proper conditional that only includes 'w-full' when width === 'full'.packages/koenig-lexical/src/plugins/HeaderPlugin.tsx (1)
11-28:⚠️ Potential issue | 🟠 MajorAdd
useEffectdependency array to prevent re-registering command on every render.The
useEffectat lines 11-28 is missing a dependency array, causing it to re-run on every render and repeatedly register/unregister the command handler. Add[editor]as the dependency array to match the pattern used in other similar plugins.💡 Proposed fix
- }); + }, [editor]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/koenig-lexical/src/plugins/HeaderPlugin.tsx` around lines 11 - 28, The useEffect in HeaderPlugin (the effect that checks editor.hasNodes(HeaderNode) and registers INSERT_HEADER_COMMAND via editor.registerCommand and mergeRegister) is missing a dependency array and therefore re-registers on every render; update that useEffect to include [editor] as its dependency so the registration/unregistration runs only when the editor instance changes, retaining the existing logic that creates the card node via $createHeaderNode and dispatches INSERT_CARD_COMMAND with COMMAND_PRIORITY_LOW.packages/koenig-lexical/src/plugins/CalloutPlugin.tsx (1)
11-28:⚠️ Potential issue | 🟠 Major
useEffectis missing the[editor]dependency array.At line 28, the effect closes without a dependency array, causing the command registration to re-run on every render. Comparable plugin files (EmbedPlugin, HtmlPlugin) include
[editor]as a dependency since the effect uses the editor object.Proposed fix
- }); + }, [editor]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/koenig-lexical/src/plugins/CalloutPlugin.tsx` around lines 11 - 28, The useEffect in CalloutPlugin is missing its dependency array which causes the command registration to run on every render; update the React.useEffect call that checks editor.hasNodes([CalloutNode]) and registers the INSERT_CALLOUT_COMMAND to include [editor] as its dependency so the effect only re-runs when the editor instance changes, ensuring proper lifecycle for editor.registerCommand and avoiding duplicate registrations for INSERT_CALLOUT_COMMAND, the $createCalloutNode call, and editor.dispatchCommand.packages/koenig-lexical/src/plugins/FilePlugin.tsx (1)
11-28:⚠️ Potential issue | 🟠 MajorAdd a dependency array to avoid repeated command re-registration.
At line 11,
React.useEffectruns after every render because it has no dependency list, which repeatedly tears down and re-registersINSERT_FILE_COMMAND. Properly implemented plugins like AudioPlugin use[editor]as the dependency array.💡 Proposed fix
- }, [editor]); + }, [editor]); });Change line 28 from
});to}, [editor]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/koenig-lexical/src/plugins/FilePlugin.tsx` around lines 11 - 28, The useEffect in FilePlugin re-registers INSERT_FILE_COMMAND on every render because it has no dependency array; update the React.useEffect call that checks editor.hasNodes(FileNode) and registers the command (the block using mergeRegister, editor.registerCommand, INSERT_FILE_COMMAND, INSERT_CARD_COMMAND and $createFileNode) to include [editor] as its dependency array so the registration is only created/cleaned when the editor instance changes.packages/koenig-lexical/src/plugins/PaywallPlugin.tsx (1)
11-14:⚠️ Potential issue | 🟠 MajorBug:
hasNodes([])with empty array doesn't validate PaywallNode registration.The check passes an empty array, so it always returns
trueand never logs the error. This should check forPaywallNode.🐛 Proposed fix
+import {$createPaywallNode, INSERT_PAYWALL_COMMAND, PaywallNode} from '../nodes/PaywallNode'; -import {$createPaywallNode, INSERT_PAYWALL_COMMAND} from '../nodes/PaywallNode';useEffect(() => { - if (!editor.hasNodes([])) { + if (!editor.hasNodes([PaywallNode])) { console.error('PaywallPlugin: PaywallNode not registered'); return; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/koenig-lexical/src/plugins/PaywallPlugin.tsx` around lines 11 - 14, The current check in PaywallPlugin uses editor.hasNodes([]) which always succeeds; change the validation to explicitly check for the PaywallNode type (e.g., use editor.hasNodes([PaywallNode]) or the correct API form) and ensure PaywallNode is imported/available, so the console.error('PaywallPlugin: PaywallNode not registered') path is reachable when the node is missing; update the condition in the PaywallPlugin initialization where editor.hasNodes([]) is called.packages/koenig-lexical/src/plugins/KoenigFocusPlugin.tsx (1)
7-16:⚠️ Potential issue | 🔴 CriticalMissing cleanup:
registerCommandreturns an unsubscribe function that must be returned fromuseEffect.The command listener is never unregistered, causing a memory leak. Each time
onFocuschanges, a new listener is added without removing the previous one.🐛 Proposed fix
useEffect(() => { - editor.registerCommand( + return editor.registerCommand( FOCUS_COMMAND, () => { onFocus?.(); return false; }, COMMAND_PRIORITY_EDITOR ); }, [editor, onFocus]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/koenig-lexical/src/plugins/KoenigFocusPlugin.tsx` around lines 7 - 16, The useEffect registering the FOCUS_COMMAND currently never unsubscribes; capture the unsubscribe returned by editor.registerCommand inside the useEffect and return it as the cleanup function so the listener is removed when onFocus or editor changes. Specifically, update the useEffect that calls editor.registerCommand(FOCUS_COMMAND, ... , COMMAND_PRIORITY_EDITOR) to store the returned disposer and return that disposer (or a function that calls it) so previous registrations are cleaned up when onFocus updates.packages/koenig-lexical/src/plugins/HtmlOutputPlugin.tsx (1)
49-53:⚠️ Potential issue | 🟡 MinorInconsistent optional chaining on
setHtml.Line 50 uses
setHtml?.(htmlString)with optional chaining, but line 52 callssetHtml('')directly. SincesetHtmlis typed as required (setHtml: (html: string) => void), the optional chaining on line 50 is unnecessary.For consistency, either:
- Remove the optional chaining since
setHtmlis required, or- If
setHtmlshould be optional, update the type and add optional chaining on line 52Suggested fix (if setHtml remains required)
if (captionText) { - setHtml?.(htmlString); + setHtml(htmlString); } else { setHtml(''); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/koenig-lexical/src/plugins/HtmlOutputPlugin.tsx` around lines 49 - 53, The conditional uses optional chaining on setHtml (setHtml?.(htmlString)) but later calls setHtml('') directly; since setHtml is declared required (setHtml: (html: string) => void) remove the unnecessary optional chaining and call setHtml(htmlString) for consistency; alternatively, if setHtml should be optional, update its type to (html?: string) => void (or setHtml?: (html: string) => void) and add optional chaining on the empty-string call as well — locate the usage in HtmlOutputPlugin.tsx around the captionText/htmlString handling and update the setHtml calls and signature accordingly.packages/koenig-lexical/src/plugins/FloatingToolbarPlugin.tsx (1)
83-100:⚠️ Potential issue | 🟠 MajorReturn the disposer from
registerCommand.The effect at lines 83-100 registers a command handler but doesn't return the cleanup function from
editor.registerCommand(). This causes handlers to accumulate across remounts, especially problematic under React Strict Mode, which intentionally invokes effects twice during development.💡 Suggested fix
React.useEffect(() => { - editor.registerCommand( + return editor.registerCommand( KEY_MODIFIER_COMMAND, (event: KeyboardEvent) => { const {keyCode, ctrlKey, metaKey, shiftKey} = event; // ctrl/cmd K with selected text should prompt for link insertion if (!shiftKey && keyCode === 75 && (ctrlKey || metaKey)) { const selection = $getSelection(); if ($isRangeSelection(selection) && !selection.isCollapsed()) { setToolbarItemType(toolbarItemTypes.link); event.preventDefault(); return true; } } return false; }, COMMAND_PRIORITY_LOW ); }, [editor]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/koenig-lexical/src/plugins/FloatingToolbarPlugin.tsx` around lines 83 - 100, The effect registers a command via editor.registerCommand (for KEY_MODIFIER_COMMAND) but never returns the disposer, causing handlers to accumulate; update the React.useEffect to capture the disposer returned by editor.registerCommand and return a cleanup function that calls that disposer (or unregisters the command) so the handler is removed on unmount/re-run—place this around the registration for KEY_MODIFIER_COMMAND and ensure setToolbarItemType usage remains unchanged inside the handler.packages/koenig-lexical/src/components/ui/UrlSearchInput.tsx (1)
51-58:⚠️ Potential issue | 🟡 MinorDon't render a dead “Paste URL as link” action.
Line 57 made the callback optional, but this button still renders for every error state. If a caller omits
handlePasteAsLink, users get a visible action that can never do anything.🩹 Proposed fix
- <button className="mr-3 cursor-pointer" data-testid={`${dataTestId}-error-pasteAsLink`} type="button"><span className="font-semibold underline" onClick={() => handlePasteAsLink?.(value)}>Paste URL as link</span></button> + {handlePasteAsLink && ( + <button + className="mr-3 cursor-pointer" + data-testid={`${dataTestId}-error-pasteAsLink`} + type="button" + onClick={() => handlePasteAsLink(value)} + > + <span className="font-semibold underline">Paste URL as link</span> + </button> + )}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/koenig-lexical/src/components/ui/UrlSearchInput.tsx` around lines 51 - 58, The error UI renders a non-functional "Paste URL as link" button even when the optional handler handlePasteAsLink is not provided; update the UrlSearchInput component so the error-state button (data-testid `${dataTestId}-error-pasteAsLink`) is only rendered when handlePasteAsLink is defined (e.g. guard the button with handlePasteAsLink) and preserve the existing markup/attributes and onClick invocation when present.packages/koenig-lexical/src/components/ui/cards/VideoCard.tsx (1)
168-176:⚠️ Potential issue | 🟡 MinorIncorrect event type cast for form
onChange.Line 168 casts
onFileChange(which expectsReact.ChangeEvent<HTMLInputElement>) toReact.FormEventHandler<HTMLFormElement>. These are different event types - form events bubble from child inputs and haveHTMLFormElementascurrentTarget, notHTMLInputElement.The pattern of using form
onChangeto capture file input changes is unconventional. Consider attachingonChangedirectly to the<input>element instead.🐛 Proposed fix
- <form onChange={onFileChange as unknown as React.FormEventHandler<HTMLFormElement>}> + <form> <input ref={fileInputRef as React.RefObject<HTMLInputElement>} accept={videoMimeTypes.join(',')} hidden={true} name="image-input" type='file' + onChange={onFileChange} /> </form>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/koenig-lexical/src/components/ui/cards/VideoCard.tsx` around lines 168 - 176, The form is incorrectly using onChange with a wrong event type cast; remove the onChange from the <form> and instead attach onFileChange (typed as React.ChangeEvent<HTMLInputElement>) directly to the <input> (the element referenced by fileInputRef) so the event currentTarget is an HTMLInputElement and no unsafe cast is required; update VideoCard.tsx to move the handler to the input, remove the cast to React.FormEventHandler<HTMLFormElement>, and keep accept, name, hidden and ref as-is.packages/koenig-lexical/src/components/ui/LinkInputWithSearch.tsx (1)
65-78:⚠️ Potential issue | 🟡 MinorIncorrect type cast for
onClickprop.Line 74 casts
onItemSelected as () => void, butonItemSelectedhas signature(item: {value: string; type?: string}) => void. The cast misrepresents the function signature. IfLinkInputSearchItempasses the item when callingonClick, this will work at runtime but the typing is incorrect.Consider properly typing the
LinkInputSearchItemcomponent'sonClickprop or using a more accurate cast that reflects the actual callback signature.♻️ Proposed fix
- onClick={onItemSelected as () => void} + onClick={() => onItemSelected(item)}Or if
LinkInputSearchItempasses the item internally:- onClick={onItemSelected as () => void} + onClick={onItemSelected as (item: {value: string; label: string}) => void}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/koenig-lexical/src/components/ui/LinkInputWithSearch.tsx` around lines 65 - 78, The onClick prop is being miscast in getItem; instead of using onItemSelected as () => void, either adjust LinkInputSearchItem's onClick prop type to accept (item: {value: string; type?: string}) => void, or change the call site in getItem to pass a zero-arg wrapper that forwards the item (e.g., onClick={() => onItemSelected(item)}), ensuring the signatures for LinkInputSearchItem.onClick and the onItemSelected callback match; update the types for LinkInputSearchItem props if you choose the first approach so TypeScript reflects the actual (item) => void signature.packages/koenig-lexical/src/components/ui/ColorOptionButtons.tsx (1)
70-74:⚠️ Potential issue | 🟡 MinorAccessibility:
<li>withonClicklacks keyboard support.The "background-image" list item has an
onClickhandler but no keyboard accessibility attributes (role="button",tabIndex,onKeyDown). This makes it inaccessible to keyboard users.♿ Proposed fix to add keyboard accessibility
-<li key='background-image' className={`mb-0 flex size-[3rem] cursor-pointer items-center justify-center rounded-full border-2 ${selectedName === name ? 'border-green' : 'border-transparent'}`} data-testid="background-image-color-button" onClick={() => onClick(name)}> +<li key='background-image' className={`mb-0 flex size-[3rem] cursor-pointer items-center justify-center rounded-full border-2 ${selectedName === name ? 'border-green' : 'border-transparent'}`} data-testid="background-image-color-button" role="button" tabIndex={0} onClick={() => onClick(name)} onKeyDown={(e) => { if (e.key === 'Enter' || e.key === ' ') { e.preventDefault(); onClick(name); } }}>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/koenig-lexical/src/components/ui/ColorOptionButtons.tsx` around lines 70 - 74, The list item with key 'background-image' uses an onClick handler but lacks keyboard accessibility; update that <li> (the element rendering the background-image option) to include role="button", tabIndex={0}, and an onKeyDown handler that calls the existing onClick(name) when Enter or Space is pressed (use the same onClick prop passed into the component), and add an appropriate aria-label or aria-pressed if needed to convey state (use selectedName to reflect selection). This will ensure keyboard users can focus and activate the background-image option the same way mouse users can.packages/koenig-lexical/src/nodes/ProductNodeComponent.tsx (1)
70-89:⚠️ Potential issue | 🟠 MajorAlways release the preview URL in a
finallyblock.If
getImageDimensions()orimgUploader.upload()rejects,imgPreviewnever clears and the blob URL is never revoked. Guard the empty-file case up front and move cleanup intofinally.🧹 Safer upload flow
const handleImgUpload = async (files: File[] | FileList) => { - const imgPreviewUrl = URL.createObjectURL(files[0]); + const fileArray = Array.from(files); + const [file] = fileArray; + if (!file) { + return; + } + + const imgPreviewUrl = URL.createObjectURL(file); setImgPreview(imgPreviewUrl); - const {width, height} = await getImageDimensions(imgPreviewUrl); - const imgUploadResult = await imgUploader.upload(Array.from(files)); - const imageUrl = imgUploadResult?.[0]?.url; - - if (imageUrl) { - editor.update(() => { - const node = $getProductNodeByKey(nodeKey); - if (!node) {return;} - node.productImageSrc = imageUrl; - node.productImageHeight = height; - node.productImageWidth = width; - }); + try { + const {width, height} = await getImageDimensions(imgPreviewUrl); + const imgUploadResult = await imgUploader.upload(fileArray); + const imageUrl = imgUploadResult?.[0]?.url; + + if (imageUrl) { + editor.update(() => { + const node = $getProductNodeByKey(nodeKey); + if (!node) {return;} + node.productImageSrc = imageUrl; + node.productImageHeight = height; + node.productImageWidth = width; + }); + } + } finally { + setImgPreview(''); + URL.revokeObjectURL(imgPreviewUrl); } - - setImgPreview(''); - URL.revokeObjectURL(imgPreviewUrl); };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/koenig-lexical/src/nodes/ProductNodeComponent.tsx` around lines 70 - 89, The handleImgUpload function currently sets a blob preview and performs async work but only clears setImgPreview and calls URL.revokeObjectURL when all awaits succeed; move the cleanup into a finally block so the preview is always cleared and the object URL always revoked even if getImageDimensions or imgUploader.upload throws. Also guard against empty files up front (return early if files.length === 0) and keep the editor.update block that sets node.productImageSrc/productImageHeight/productImageWidth via $getProductNodeByKey inside the try path; ensure you reference handleImgUpload, getImageDimensions, imgUploader.upload, setImgPreview, URL.revokeObjectURL and $getProductNodeByKey when making the change.packages/koenig-lexical/src/components/ui/KeyboardSelectionWithGroups.tsx (1)
41-63:⚠️ Potential issue | 🟠 MajorKeep Enter consistent with the new hover guard.
Hover now refuses to select items without
value, but the Enter path still callsonSelect(items[selectedIndex])even when the list is empty or the current item is non-selectable.⌨️ Match the keyboard path to the hover rule
if (event.key === 'Enter') { event.preventDefault(); event.stopPropagation(); - onSelect(items[selectedIndex]); + const item = items[selectedIndex]; + if (item?.value) { + onSelect(item); + } }Also applies to: 81-84
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/koenig-lexical/src/components/ui/KeyboardSelectionWithGroups.tsx` around lines 41 - 63, In handleKeydown (the callback handling ArrowUp/ArrowDown/Enter) ensure the Enter path mirrors the hover guard by verifying the selected index is in range and the targeted item has a selectable value before calling onSelect; specifically, replace the direct call onSelect(items[selectedIndex]) with a guard that checks items.length > 0, selectedIndex >= 0 and < items.length, and that items[selectedIndex].value is present/truthy, and only then call onSelect; apply the same additional guard logic to the other Enter-handling occurrence around the code referenced (lines ~81-84) so Enter never selects when the list is empty or the current item is non-selectable.packages/koenig-lexical/src/components/ui/cards/CallToActionCard.tsx (2)
417-433:⚠️ Potential issue | 🟡 MinorNon-null assertion on
htmlEditormay cause runtime error.Similar to the sponsor label editor,
htmlEditor!at line 421 assumes the editor is always provided, but it's typed as optional. TheKoenigNestedEditorcomponent likely requires a valid editor instance.🛡️ Suggested fix: Guard the rendering
-{/* HTML content */} -<KoenigNestedEditor +{/* HTML content */} +{htmlEditor && <KoenigNestedEditor autoFocus={true} dataTestId={'cta-card-content-editor'} hasSettingsPanel={true} - initialEditor={htmlEditor!} + initialEditor={htmlEditor} ... > <ReplacementStringsPlugin /> -</KoenigNestedEditor> +</KoenigNestedEditor>}Or make
htmlEditorrequired in the interface if it's always expected.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/koenig-lexical/src/components/ui/cards/CallToActionCard.tsx` around lines 417 - 433, The code uses a non-null assertion htmlEditor! when rendering KoenigNestedEditor which can crash if htmlEditor is undefined; update the render to guard this usage by only rendering KoenigNestedEditor when htmlEditor is defined (check htmlEditor before rendering) or alternatively make htmlEditor required in the component props/interface so callers must always provide it; ensure related props like htmlEditorInitialState and initialTheme are also only passed/used when htmlEditor exists to avoid further runtime errors.
370-384:⚠️ Potential issue | 🟡 MinorNon-null assertion on
sponsorLabelHtmlEditormay cause runtime error.The assertion
sponsorLabelHtmlEditor!at line 374 assumes the editor is always provided whenhasSponsorLabelis true. However, the prop is typed as optional (sponsorLabelHtmlEditor?: LexicalEditor). If the parent component passeshasSponsorLabel={true}without providing the editor, this will throw.Consider adding a guard or making the prop required when
hasSponsorLabelis true:🛡️ Option 1: Add a guard
{/* Sponsor label */} -{hasSponsorLabel && ( +{hasSponsorLabel && sponsorLabelHtmlEditor && ( <div className={clsx( 'py-3', {'mx-6': color !== 'none'} )}> <KoenigNestedEditor autoFocus={true} dataTestId={'sponsor-label-editor'} hasSettingsPanel={true} - initialEditor={sponsorLabelHtmlEditor!} + initialEditor={sponsorLabelHtmlEditor}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/koenig-lexical/src/components/ui/cards/CallToActionCard.tsx` around lines 370 - 384, The code uses a non-null assertion on sponsorLabelHtmlEditor when rendering KoenigNestedEditor which can throw if hasSponsorLabel is true but sponsorLabelHtmlEditor is undefined; update the render logic in CallToActionCard to guard before using sponsorLabelHtmlEditor (and related sponsorLabelHtmlEditorInitialState) — e.g. only render KoenigNestedEditor when hasSponsorLabel && sponsorLabelHtmlEditor are truthy, or make sponsorLabelHtmlEditor a required prop when hasSponsorLabel is true; ensure RestrictContentPlugin and props (autoFocus, initialEditor, initialEditorState, initialTheme, textClassName) are passed only after the guard so no runtime null assertion occurs.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b635288d-adc7-4a80-a19f-c848467913ba
📒 Files selected for processing (300)
packages/koenig-lexical/.storybook/main.tspackages/koenig-lexical/demo/DemoApp.tsxpackages/koenig-lexical/demo/HtmlOutputDemo.tsxpackages/koenig-lexical/demo/RestrictedContentDemo.tsxpackages/koenig-lexical/demo/components/DarkModeToggle.tsxpackages/koenig-lexical/demo/components/EmailEditorWrapper.tsxpackages/koenig-lexical/demo/components/FloatingButton.tsxpackages/koenig-lexical/demo/components/InitialContentToggle.tsxpackages/koenig-lexical/demo/components/Navigator.tsxpackages/koenig-lexical/demo/components/SerializedStateTextarea.tsxpackages/koenig-lexical/demo/components/Sidebar.tsxpackages/koenig-lexical/demo/components/TitleTextBox.tsxpackages/koenig-lexical/demo/components/TreeView.tsxpackages/koenig-lexical/demo/components/Watermark.tsxpackages/koenig-lexical/demo/components/WordCount.tsxpackages/koenig-lexical/demo/demo.tsxpackages/koenig-lexical/demo/utils/fetchEmbed.tspackages/koenig-lexical/demo/utils/tenorConfig.tspackages/koenig-lexical/demo/utils/unsplashConfig.tspackages/koenig-lexical/demo/utils/useFileUpload.tspackages/koenig-lexical/demo/utils/useSnippets.tspackages/koenig-lexical/eslint.config.jspackages/koenig-lexical/package.jsonpackages/koenig-lexical/playwright.config.tspackages/koenig-lexical/src/components/DesignSandbox.tsxpackages/koenig-lexical/src/components/KoenigCaptionEditor.tsxpackages/koenig-lexical/src/components/KoenigCardWrapper.tsxpackages/koenig-lexical/src/components/KoenigComposableEditor.tsxpackages/koenig-lexical/src/components/KoenigComposer.tsxpackages/koenig-lexical/src/components/KoenigEditor.tsxpackages/koenig-lexical/src/components/KoenigErrorBoundary.tsxpackages/koenig-lexical/src/components/KoenigNestedComposer.tsxpackages/koenig-lexical/src/components/KoenigNestedEditor.tsxpackages/koenig-lexical/src/components/ui/ActionToolbar.tsxpackages/koenig-lexical/src/components/ui/AtLinkResultsPopup.tsxpackages/koenig-lexical/src/components/ui/AudioUploadForm.jsxpackages/koenig-lexical/src/components/ui/AudioUploadForm.tsxpackages/koenig-lexical/src/components/ui/BackgroundImagePicker.tsxpackages/koenig-lexical/src/components/ui/Button.stories.tsxpackages/koenig-lexical/src/components/ui/Button.tsxpackages/koenig-lexical/src/components/ui/ButtonGroup.stories.tsxpackages/koenig-lexical/src/components/ui/ButtonGroup.tsxpackages/koenig-lexical/src/components/ui/CardCaptionEditor.tsxpackages/koenig-lexical/src/components/ui/CardMenu.stories.tsxpackages/koenig-lexical/src/components/ui/CardMenu.tsxpackages/koenig-lexical/src/components/ui/CardVisibilityMessage.tsxpackages/koenig-lexical/src/components/ui/CardWrapper.tsxpackages/koenig-lexical/src/components/ui/ColorOptionButtons.tsxpackages/koenig-lexical/src/components/ui/ColorPicker.stories.tsxpackages/koenig-lexical/src/components/ui/ColorPicker.tsxpackages/koenig-lexical/src/components/ui/Delayed.tsxpackages/koenig-lexical/src/components/ui/Dropdown.stories.tsxpackages/koenig-lexical/src/components/ui/Dropdown.tsxpackages/koenig-lexical/src/components/ui/DropdownContainer.tsxpackages/koenig-lexical/src/components/ui/EditorPlaceholder.tsxpackages/koenig-lexical/src/components/ui/EmojiPicker.tsxpackages/koenig-lexical/src/components/ui/EmojiPickerPortal.tsxpackages/koenig-lexical/src/components/ui/FileUploadForm.jsxpackages/koenig-lexical/src/components/ui/FileUploadForm.tsxpackages/koenig-lexical/src/components/ui/FloatingFormatToolbar.tsxpackages/koenig-lexical/src/components/ui/FloatingLinkToolbar.tsxpackages/koenig-lexical/src/components/ui/FloatingToolbar.tsxpackages/koenig-lexical/src/components/ui/FormatToolbar.tsxpackages/koenig-lexical/src/components/ui/HighlightedString.tsxpackages/koenig-lexical/src/components/ui/IconButton.stories.tsxpackages/koenig-lexical/src/components/ui/IconButton.tsxpackages/koenig-lexical/src/components/ui/ImageUploadForm.tsxpackages/koenig-lexical/src/components/ui/ImageUploadSwatch.tsxpackages/koenig-lexical/src/components/ui/Input.stories.tsxpackages/koenig-lexical/src/components/ui/Input.tsxpackages/koenig-lexical/src/components/ui/InputList.stories.tsxpackages/koenig-lexical/src/components/ui/InputList.tsxpackages/koenig-lexical/src/components/ui/KeyboardSelection.tsxpackages/koenig-lexical/src/components/ui/KeyboardSelectionWithGroups.tsxpackages/koenig-lexical/src/components/ui/LinkActionToolbar.tsxpackages/koenig-lexical/src/components/ui/LinkActionToolbarWithSearch.tsxpackages/koenig-lexical/src/components/ui/LinkInput.stories.tsxpackages/koenig-lexical/src/components/ui/LinkInput.tsxpackages/koenig-lexical/src/components/ui/LinkInputSearchItem.tsxpackages/koenig-lexical/src/components/ui/LinkInputWithSearch.stories.tsxpackages/koenig-lexical/src/components/ui/LinkInputWithSearch.tsxpackages/koenig-lexical/src/components/ui/LinkToolbar.stories.tsxpackages/koenig-lexical/src/components/ui/LinkToolbar.tsxpackages/koenig-lexical/src/components/ui/MediaPlaceholder.stories.tsxpackages/koenig-lexical/src/components/ui/MediaPlaceholder.tsxpackages/koenig-lexical/src/components/ui/MediaPlayer.stories.tsxpackages/koenig-lexical/src/components/ui/MediaPlayer.tsxpackages/koenig-lexical/src/components/ui/MediaUploader.tsxpackages/koenig-lexical/src/components/ui/Modal.stories.tsxpackages/koenig-lexical/src/components/ui/Modal.tsxpackages/koenig-lexical/src/components/ui/MultiSelectDropdown.tsxpackages/koenig-lexical/src/components/ui/PlusMenu.stories.tsxpackages/koenig-lexical/src/components/ui/PlusMenu.tsxpackages/koenig-lexical/src/components/ui/Portal.tsxpackages/koenig-lexical/src/components/ui/ProgressBar.stories.tsxpackages/koenig-lexical/src/components/ui/ProgressBar.tsxpackages/koenig-lexical/src/components/ui/ReadOnlyOverlay.tsxpackages/koenig-lexical/src/components/ui/SettingsPanel.stories.tsxpackages/koenig-lexical/src/components/ui/SettingsPanel.tsxpackages/koenig-lexical/src/components/ui/SlashMenu.jsxpackages/koenig-lexical/src/components/ui/SlashMenu.tsxpackages/koenig-lexical/src/components/ui/Slider.stories.tsxpackages/koenig-lexical/src/components/ui/Slider.tsxpackages/koenig-lexical/src/components/ui/SnippetActionToolbar.tsxpackages/koenig-lexical/src/components/ui/SnippetInput.stories.tsxpackages/koenig-lexical/src/components/ui/SnippetInput.tsxpackages/koenig-lexical/src/components/ui/SnippetInput/Dropdown.tsxpackages/koenig-lexical/src/components/ui/SnippetInput/Input.tsxpackages/koenig-lexical/src/components/ui/Spinner.tsxpackages/koenig-lexical/src/components/ui/SubscribeForm.stories.tsxpackages/koenig-lexical/src/components/ui/SubscribeForm.tsxpackages/koenig-lexical/src/components/ui/TabView.tsxpackages/koenig-lexical/src/components/ui/TenorPlugin.tsxpackages/koenig-lexical/src/components/ui/TenorSelector.stories.tsxpackages/koenig-lexical/src/components/ui/TenorSelector.tsxpackages/koenig-lexical/src/components/ui/TextInput.tsxpackages/koenig-lexical/src/components/ui/Toggle.stories.tsxpackages/koenig-lexical/src/components/ui/Toggle.tsxpackages/koenig-lexical/src/components/ui/ToolbarMenu.stories.tsxpackages/koenig-lexical/src/components/ui/ToolbarMenu.tsxpackages/koenig-lexical/src/components/ui/ToolbarMenuItem.stories.tsxpackages/koenig-lexical/src/components/ui/Tooltip.tsxpackages/koenig-lexical/src/components/ui/UnsplashPlugin.tsxpackages/koenig-lexical/src/components/ui/UrlInput.stories.tsxpackages/koenig-lexical/src/components/ui/UrlInput.tsxpackages/koenig-lexical/src/components/ui/UrlSearchInput.stories.tsxpackages/koenig-lexical/src/components/ui/UrlSearchInput.tsxpackages/koenig-lexical/src/components/ui/VisibilitySettings.tsxpackages/koenig-lexical/src/components/ui/cards/AudioCard.stories.tsxpackages/koenig-lexical/src/components/ui/cards/AudioCard.tsxpackages/koenig-lexical/src/components/ui/cards/BookmarkCard.stories.tsxpackages/koenig-lexical/src/components/ui/cards/BookmarkCard.tsxpackages/koenig-lexical/src/components/ui/cards/ButtonCard.stories.tsxpackages/koenig-lexical/src/components/ui/cards/ButtonCard.tsxpackages/koenig-lexical/src/components/ui/cards/CallToActionCard.stories.tsxpackages/koenig-lexical/src/components/ui/cards/CallToActionCard.tsxpackages/koenig-lexical/src/components/ui/cards/CalloutCard.stories.tsxpackages/koenig-lexical/src/components/ui/cards/CalloutCard.tsxpackages/koenig-lexical/src/components/ui/cards/CodeBlockCard.jsxpackages/koenig-lexical/src/components/ui/cards/CodeBlockCard.stories.tsxpackages/koenig-lexical/src/components/ui/cards/CodeBlockCard.tsxpackages/koenig-lexical/src/components/ui/cards/EmailCard.stories.tsxpackages/koenig-lexical/src/components/ui/cards/EmailCard.tsxpackages/koenig-lexical/src/components/ui/cards/EmailCtaCard.stories.tsxpackages/koenig-lexical/src/components/ui/cards/EmailCtaCard.tsxpackages/koenig-lexical/src/components/ui/cards/EmbedCard.stories.tsxpackages/koenig-lexical/src/components/ui/cards/EmbedCard.tsxpackages/koenig-lexical/src/components/ui/cards/FileCard.stories.tsxpackages/koenig-lexical/src/components/ui/cards/FileCard.tsxpackages/koenig-lexical/src/components/ui/cards/GalleryCard.stories.tsxpackages/koenig-lexical/src/components/ui/cards/GalleryCard.tsxpackages/koenig-lexical/src/components/ui/cards/HeaderCard/v1/HeaderCard.tsxpackages/koenig-lexical/src/components/ui/cards/HeaderCard/v1/HeaderCard.v1.stories.tsxpackages/koenig-lexical/src/components/ui/cards/HeaderCard/v2/HeaderCard.tsxpackages/koenig-lexical/src/components/ui/cards/HeaderCard/v2/HeaderCard.v2.stories.tsxpackages/koenig-lexical/src/components/ui/cards/HorizontalRuleCard.stories.tsxpackages/koenig-lexical/src/components/ui/cards/HorizontalRuleCard.tsxpackages/koenig-lexical/src/components/ui/cards/HtmlCard.stories.tsxpackages/koenig-lexical/src/components/ui/cards/HtmlCard.tsxpackages/koenig-lexical/src/components/ui/cards/HtmlCard/HtmlEditor.tsxpackages/koenig-lexical/src/components/ui/cards/ImageCard.stories.tsxpackages/koenig-lexical/src/components/ui/cards/ImageCard.tsxpackages/koenig-lexical/src/components/ui/cards/MarkdownCard.stories.tsxpackages/koenig-lexical/src/components/ui/cards/MarkdownCard.tsxpackages/koenig-lexical/src/components/ui/cards/MarkdownCard/MarkdownEditor.tsxpackages/koenig-lexical/src/components/ui/cards/MarkdownCard/MarkdownHelpDialog.tsxpackages/koenig-lexical/src/components/ui/cards/MarkdownCard/MarkdownImageUploader.tsxpackages/koenig-lexical/src/components/ui/cards/MarkdownCard/useMarkdownImageUploader.tspackages/koenig-lexical/src/components/ui/cards/PaywallCard.stories.tsxpackages/koenig-lexical/src/components/ui/cards/PaywallCard.tsxpackages/koenig-lexical/src/components/ui/cards/ProductCard.stories.tsxpackages/koenig-lexical/src/components/ui/cards/ProductCard.tsxpackages/koenig-lexical/src/components/ui/cards/ProductCard/ProductCardImage.tsxpackages/koenig-lexical/src/components/ui/cards/ProductCard/RatingButton.tsxpackages/koenig-lexical/src/components/ui/cards/SignupCard.stories.tsxpackages/koenig-lexical/src/components/ui/cards/SignupCard.tsxpackages/koenig-lexical/src/components/ui/cards/ToggleCard.stories.tsxpackages/koenig-lexical/src/components/ui/cards/ToggleCard.tsxpackages/koenig-lexical/src/components/ui/cards/TransistorCard.tsxpackages/koenig-lexical/src/components/ui/cards/VideoCard.stories.tsxpackages/koenig-lexical/src/components/ui/cards/VideoCard.tsxpackages/koenig-lexical/src/components/ui/file-selectors/Tenor/Error.tsxpackages/koenig-lexical/src/components/ui/file-selectors/Tenor/Gif.tsxpackages/koenig-lexical/src/components/ui/file-selectors/Tenor/Loader.tsxpackages/koenig-lexical/src/components/ui/file-selectors/Unsplash/UnsplashButton.tsxpackages/koenig-lexical/src/components/ui/file-selectors/Unsplash/UnsplashGallery.tsxpackages/koenig-lexical/src/components/ui/file-selectors/Unsplash/UnsplashImage.tsxpackages/koenig-lexical/src/components/ui/file-selectors/Unsplash/UnsplashSelector.stories.tsxpackages/koenig-lexical/src/components/ui/file-selectors/Unsplash/UnsplashSelector.tsxpackages/koenig-lexical/src/components/ui/file-selectors/Unsplash/UnsplashZoomed.tsxpackages/koenig-lexical/src/components/ui/file-selectors/UnsplashModal.jsxpackages/koenig-lexical/src/components/ui/file-selectors/UnsplashModal.tsxpackages/koenig-lexical/src/context/CardContext.jsxpackages/koenig-lexical/src/context/CardContext.tsxpackages/koenig-lexical/src/context/KoenigComposerContext.jsxpackages/koenig-lexical/src/context/KoenigComposerContext.tsxpackages/koenig-lexical/src/context/KoenigSelectedCardContext.tsxpackages/koenig-lexical/src/context/SharedHistoryContext.tsxpackages/koenig-lexical/src/context/SharedOnChangeContext.jsxpackages/koenig-lexical/src/context/SharedOnChangeContext.tsxpackages/koenig-lexical/src/context/TKContext.tsxpackages/koenig-lexical/src/hooks/useCardDragAndDrop.tspackages/koenig-lexical/src/hooks/useClickOutside.tspackages/koenig-lexical/src/hooks/useFileDragAndDrop.tspackages/koenig-lexical/src/hooks/useGalleryReorder.tspackages/koenig-lexical/src/hooks/useInputSelection.tspackages/koenig-lexical/src/hooks/useKoenigTextEntity.tspackages/koenig-lexical/src/hooks/useMovable.tspackages/koenig-lexical/src/hooks/usePinturaEditor.tspackages/koenig-lexical/src/hooks/usePreviousFocus.jspackages/koenig-lexical/src/hooks/usePreviousFocus.tspackages/koenig-lexical/src/hooks/useSearchLinks.tspackages/koenig-lexical/src/hooks/useSettingsPanelReposition.tspackages/koenig-lexical/src/hooks/useTypeaheadTriggerMatch.tspackages/koenig-lexical/src/hooks/useVisibilityToggle.tspackages/koenig-lexical/src/index.tspackages/koenig-lexical/src/nodes/AsideNode.tspackages/koenig-lexical/src/nodes/AudioNode.tsxpackages/koenig-lexical/src/nodes/AudioNodeComponent.tsxpackages/koenig-lexical/src/nodes/BasicNodes.tspackages/koenig-lexical/src/nodes/BookmarkNode.tsxpackages/koenig-lexical/src/nodes/BookmarkNodeComponent.tsxpackages/koenig-lexical/src/nodes/ButtonNode.tsxpackages/koenig-lexical/src/nodes/ButtonNodeComponent.tsxpackages/koenig-lexical/src/nodes/CallToActionNode.tsxpackages/koenig-lexical/src/nodes/CallToActionNodeComponent.tsxpackages/koenig-lexical/src/nodes/CalloutNode.tsxpackages/koenig-lexical/src/nodes/CalloutNodeComponent.tsxpackages/koenig-lexical/src/nodes/CodeBlockNode.tsxpackages/koenig-lexical/src/nodes/CodeBlockNodeComponent.tsxpackages/koenig-lexical/src/nodes/DefaultNodes.tspackages/koenig-lexical/src/nodes/EmailCtaNode.tsxpackages/koenig-lexical/src/nodes/EmailCtaNodeComponent.tsxpackages/koenig-lexical/src/nodes/EmailEditorNodes.tspackages/koenig-lexical/src/nodes/EmailEmbedNode.tsxpackages/koenig-lexical/src/nodes/EmailNode.tsxpackages/koenig-lexical/src/nodes/EmailNodeComponent.tsxpackages/koenig-lexical/src/nodes/EmailNodes.tspackages/koenig-lexical/src/nodes/EmbedNode.tsxpackages/koenig-lexical/src/nodes/EmbedNodeComponent.tsxpackages/koenig-lexical/src/nodes/FileNode.tsxpackages/koenig-lexical/src/nodes/FileNodeComponent.tsxpackages/koenig-lexical/src/nodes/GalleryNode.tsxpackages/koenig-lexical/src/nodes/GalleryNodeComponent.tsxpackages/koenig-lexical/src/nodes/HeaderNode.tsxpackages/koenig-lexical/src/nodes/HorizontalRuleNode.tsxpackages/koenig-lexical/src/nodes/HtmlNode.tsxpackages/koenig-lexical/src/nodes/HtmlNodeComponent.tsxpackages/koenig-lexical/src/nodes/ImageNode.tsxpackages/koenig-lexical/src/nodes/ImageNodeComponent.tsxpackages/koenig-lexical/src/nodes/MarkdownNode.tsxpackages/koenig-lexical/src/nodes/MarkdownNodeComponent.tsxpackages/koenig-lexical/src/nodes/MinimalNodes.tspackages/koenig-lexical/src/nodes/PaywallNode.tsxpackages/koenig-lexical/src/nodes/ProductNode.tsxpackages/koenig-lexical/src/nodes/ProductNodeComponent.tsxpackages/koenig-lexical/src/nodes/SignupNode.tsxpackages/koenig-lexical/src/nodes/SignupNodeComponent.tsxpackages/koenig-lexical/src/nodes/ToggleNode.tsxpackages/koenig-lexical/src/nodes/ToggleNodeComponent.tsxpackages/koenig-lexical/src/nodes/TransistorNode.tsxpackages/koenig-lexical/src/nodes/TransistorNodeComponent.tsxpackages/koenig-lexical/src/nodes/VideoNode.tsxpackages/koenig-lexical/src/nodes/VideoNodeComponent.tsxpackages/koenig-lexical/src/nodes/header/v1/HeaderNodeComponent.tsxpackages/koenig-lexical/src/nodes/header/v2/HeaderNodeComponent.tsxpackages/koenig-lexical/src/plugins/AllDefaultPlugins.tsxpackages/koenig-lexical/src/plugins/AtLinkPlugin.tsxpackages/koenig-lexical/src/plugins/AudioPlugin.tsxpackages/koenig-lexical/src/plugins/BookmarkPlugin.tsxpackages/koenig-lexical/src/plugins/ButtonPlugin.tsxpackages/koenig-lexical/src/plugins/CallToActionPlugin.tsxpackages/koenig-lexical/src/plugins/CalloutPlugin.tsxpackages/koenig-lexical/src/plugins/CardMenuPlugin.tsxpackages/koenig-lexical/src/plugins/DragDropPastePlugin.tsxpackages/koenig-lexical/src/plugins/DragDropReorderPlugin.tsxpackages/koenig-lexical/src/plugins/EmEnDashPlugin.tsxpackages/koenig-lexical/src/plugins/EmailCtaPlugin.tsxpackages/koenig-lexical/src/plugins/EmailPlugin.tsxpackages/koenig-lexical/src/plugins/EmbedPlugin.tsxpackages/koenig-lexical/src/plugins/EmojiPickerPlugin.tsxpackages/koenig-lexical/src/plugins/ExternalControlPlugin.tsxpackages/koenig-lexical/src/plugins/FilePlugin.tsxpackages/koenig-lexical/src/plugins/FloatingToolbarPlugin.tsxpackages/koenig-lexical/src/plugins/GalleryPlugin.tsxpackages/koenig-lexical/src/plugins/HeaderPlugin.tsxpackages/koenig-lexical/src/plugins/HorizontalRulePlugin.tsxpackages/koenig-lexical/src/plugins/HtmlOutputPlugin.tsxpackages/koenig-lexical/src/plugins/HtmlPlugin.tsxpackages/koenig-lexical/src/plugins/ImagePlugin.tsxpackages/koenig-lexical/src/plugins/KoenigBehaviourPlugin.tsxpackages/koenig-lexical/src/plugins/KoenigBlurPlugin.tsxpackages/koenig-lexical/src/plugins/KoenigFocusPlugin.tsxpackages/koenig-lexical/src/plugins/KoenigNestedEditorPlugin.tsxpackages/koenig-lexical/src/plugins/KoenigSelectorPlugin.tsxpackages/koenig-lexical/src/plugins/KoenigSnippetPlugin.tsxpackages/koenig-lexical/src/plugins/MarkdownPastePlugin.tsxpackages/koenig-lexical/src/plugins/MarkdownPlugin.tsxpackages/koenig-lexical/src/plugins/MarkdownShortcutPlugin.tsxpackages/koenig-lexical/src/plugins/PaywallPlugin.tsx
💤 Files with no reviewable changes (9)
- packages/koenig-lexical/src/components/ui/SlashMenu.jsx
- packages/koenig-lexical/src/components/ui/file-selectors/UnsplashModal.jsx
- packages/koenig-lexical/src/context/CardContext.jsx
- packages/koenig-lexical/src/context/KoenigComposerContext.jsx
- packages/koenig-lexical/src/components/ui/AudioUploadForm.jsx
- packages/koenig-lexical/src/components/ui/FileUploadForm.jsx
- packages/koenig-lexical/src/components/ui/cards/CodeBlockCard.jsx
- packages/koenig-lexical/src/context/SharedOnChangeContext.jsx
- packages/koenig-lexical/src/hooks/usePreviousFocus.js
| interface SidebarProps { | ||
| isOpen: boolean; | ||
| view: string; | ||
| saveContent?: () => void; | ||
| } | ||
|
|
||
| const Sidebar = ({isOpen, view, saveContent}: SidebarProps) => { |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify Sidebar call sites pass valid literal views and correct saveContent pairing.
rg -n -C2 '<Sidebar\b|view=\{?["'\'']?(json|tree)|saveContent=' --type=tsx --type=jsxRepository: TryGhost/Koenig
Length of output: 86
🏁 Script executed:
# Search for Sidebar component usage with corrected flags
rg -n '<Sidebar' --glob='*.{tsx,jsx}' -C2Repository: TryGhost/Koenig
Length of output: 1765
🏁 Script executed:
# Also read the full Sidebar.tsx file to understand the implementation
cat packages/koenig-lexical/demo/components/Sidebar.tsxRepository: TryGhost/Koenig
Length of output: 941
🏁 Script executed:
# Check RestrictedContentDemo for sidebarView initialization/values
rg -n 'sidebarView' packages/koenig-lexical/demo/RestrictedContentDemo.tsx -B3 -A3Repository: TryGhost/Koenig
Length of output: 1086
🏁 Script executed:
# Check HtmlOutputDemo for sidebarView initialization/values
rg -n 'sidebarView' packages/koenig-lexical/demo/HtmlOutputDemo.tsx -B3 -A3Repository: TryGhost/Koenig
Length of output: 1245
🏁 Script executed:
# Check DemoApp for sidebarView initialization/values
rg -n 'sidebarView' packages/koenig-lexical/demo/DemoApp.tsx -B3 -A3Repository: TryGhost/Koenig
Length of output: 1164
🏁 Script executed:
# Check if openSidebar is ever called with a different view in RestrictedContentDemo
rg -n 'openSidebar' packages/koenig-lexical/demo/RestrictedContentDemo.tsxRepository: TryGhost/Koenig
Length of output: 191
🏁 Script executed:
# Check if openSidebar is ever called with a different view in HtmlOutputDemo
rg -n 'openSidebar' packages/koenig-lexical/demo/HtmlOutputDemo.tsxRepository: TryGhost/Koenig
Length of output: 195
🏁 Script executed:
# Check what FloatingButton does - does it ever call openSidebar with a 'tree' argument
rg -n 'FloatingButton' packages/koenig-lexical/demo/components/ -A20Repository: TryGhost/Koenig
Length of output: 2052
🏁 Script executed:
# Also verify the actual FloatingButton component implementation
find packages/koenig-lexical/demo -name 'FloatingButton*'Repository: TryGhost/Koenig
Length of output: 117
Fix runtime bug in RestrictedContentDemo and HtmlOutputDemo by refactoring SidebarProps with a discriminated union.
Currently, RestrictedContentDemo.tsx (line 117) and HtmlOutputDemo.tsx (line 111) pass Sidebar without saveContent, but when users click the "JSON output" button, the component attempts to render <button onClick={saveContent}> with an undefined callback, causing a runtime error. Refactor SidebarProps as a discriminated union to enforce that saveContent is required when view is 'json' and forbidden when it is 'tree'.
♻️ Proposed typing refactor
-interface SidebarProps {
- isOpen: boolean;
- view: string;
- saveContent?: () => void;
-}
+type SidebarProps =
+ | {isOpen: boolean; view: 'json'; saveContent: () => void}
+ | {isOpen: boolean; view: 'tree'; saveContent?: never};🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/koenig-lexical/demo/components/Sidebar.tsx` around lines 4 - 10,
SidebarProps currently allows absence of saveContent causing a runtime error
when Sidebar (component Sidebar) is rendered with view === 'json' but
saveContent is undefined; refactor SidebarProps into a discriminated union: one
variant { view: 'json'; isOpen: boolean; saveContent: () => void } and another
variant { view: 'tree'; isOpen: boolean } so the type system enforces
saveContent is required for 'json' and forbidden for 'tree'; update the Sidebar
component signature to use this union and narrow by view before calling
saveContent, and update callers (RestrictedContentDemo and HtmlOutputDemo) to
pass saveContent when they render Sidebar with view='json' or switch their view
to 'tree' to avoid passing saveContent.
| import React, {useState} from 'react'; | ||
| import {Modal} from './Modal'; | ||
| import {useState} from 'react'; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n packages/koenig-lexical/src/components/ui/Modal.stories.tsx | head -30Repository: TryGhost/Koenig
Length of output: 1004
Remove duplicate useState import to avoid module-scope redeclaration.
useState is imported on Line 2 and again on Line 4, which causes a duplicate identifier error in TS/ESM parsing.
Proposed fix
import React, {useState} from 'react';
import {Modal} from './Modal';
-import {useState} from 'react';📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import React, {useState} from 'react'; | |
| import {Modal} from './Modal'; | |
| import {useState} from 'react'; | |
| import React, {useState} from 'react'; | |
| import {Modal} from './Modal'; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/koenig-lexical/src/components/ui/Modal.stories.tsx` around lines 2 -
4, The file imports useState twice causing a module-scope redeclaration; remove
the duplicate import statement (the second "import {useState} from 'react';")
and ensure useState is only imported once from React (e.g., keep "import React,
{useState} from 'react';") so references in Modal.stories.tsx and the Modal
component remain valid.
| function highlightPrev() { | ||
| if (highlightedGif.index === 0) { | ||
| if (highlightedGif!.index === 0) { | ||
| // reached the beginning, focus the search bar | ||
| focusSearch(); | ||
| } | ||
|
|
||
| setHighlightedGif(gifs[highlightedGif.index - 1]); | ||
| setHighlightedGif(gifs[highlightedGif!.index - 1]); | ||
| } |
There was a problem hiding this comment.
highlightPrev accesses gifs[-1] when index is 0.
When highlightedGif.index === 0, the function calls focusSearch() but does not return early. Execution continues to line 138 which accesses gifs[0 - 1] = gifs[-1], resulting in undefined being set as the highlighted GIF.
Add early return after focusSearch()
function highlightPrev() {
if (highlightedGif!.index === 0) {
// reached the beginning, focus the search bar
focusSearch();
+ return;
}
setHighlightedGif(gifs[highlightedGif!.index - 1]);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function highlightPrev() { | |
| if (highlightedGif.index === 0) { | |
| if (highlightedGif!.index === 0) { | |
| // reached the beginning, focus the search bar | |
| focusSearch(); | |
| } | |
| setHighlightedGif(gifs[highlightedGif.index - 1]); | |
| setHighlightedGif(gifs[highlightedGif!.index - 1]); | |
| } | |
| function highlightPrev() { | |
| if (highlightedGif!.index === 0) { | |
| // reached the beginning, focus the search bar | |
| focusSearch(); | |
| return; | |
| } | |
| setHighlightedGif(gifs[highlightedGif!.index - 1]); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/koenig-lexical/src/components/ui/TenorSelector.tsx` around lines 132
- 139, The bug is that highlightPrev() calls focusSearch() when
highlightedGif!.index === 0 but then continues and does
setHighlightedGif(gifs[highlightedGif!.index - 1]) which reads gifs[-1]; fix
highlightPrev() so that after calling focusSearch() it returns early to avoid
accessing a negative index, i.e. inside highlightPrev() check highlightedGif (or
keep the existing non-null assertion), call focusSearch() when
highlightedGif.index === 0 and immediately return; otherwise call
setHighlightedGif(gifs[highlightedGif.index - 1]).
| useEffect(() => { | ||
| editor.registerCommand( | ||
| BLUR_COMMAND, | ||
| () => { | ||
| onBlur?.(); | ||
| return false; | ||
| }, | ||
| COMMAND_PRIORITY_EDITOR | ||
| ); |
There was a problem hiding this comment.
Missing cleanup for command registration causes memory leak.
editor.registerCommand returns an unregister function that must be returned from the useEffect to properly clean up when the component unmounts or dependencies change. Without this, the command handler remains registered.
🐛 Proposed fix
useEffect(() => {
- editor.registerCommand(
+ return editor.registerCommand(
BLUR_COMMAND,
() => {
onBlur?.();
return false;
},
COMMAND_PRIORITY_EDITOR
);
}, [editor, onBlur]);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| useEffect(() => { | |
| editor.registerCommand( | |
| BLUR_COMMAND, | |
| () => { | |
| onBlur?.(); | |
| return false; | |
| }, | |
| COMMAND_PRIORITY_EDITOR | |
| ); | |
| useEffect(() => { | |
| return editor.registerCommand( | |
| BLUR_COMMAND, | |
| () => { | |
| onBlur?.(); | |
| return false; | |
| }, | |
| COMMAND_PRIORITY_EDITOR | |
| ); | |
| }, [editor, onBlur]); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/koenig-lexical/src/plugins/KoenigBlurPlugin.tsx` around lines 7 -
15, The useEffect registers BLUR_COMMAND via editor.registerCommand (with
COMMAND_PRIORITY_EDITOR) but never cleans up, causing a leak; update the
useEffect that calls editor.registerCommand to capture the unregister function
it returns and return a cleanup function that calls it (ensuring onBlur and the
registration are removed when the component unmounts or dependencies change),
referencing the same BLUR_COMMAND/COMMAND_PRIORITY_EDITOR registration so the
exact handler is unregistered.
There was a problem hiding this comment.
Actionable comments posted: 9
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (7)
packages/koenig-lexical/src/hooks/useCardDragAndDrop.ts (1)
85-120:⚠️ Potential issue | 🟠 MajorDestroy the registered drag container on effect cleanup.
This effect re-registers a container whenever its dependencies change, but it never calls
destroy(). That leaves stale listeners behind and can fire duplicate drag/drop callbacks after rerenders.💡 Suggested fix
React.useEffect(() => { if (!containerRef || !koenig.dragDropHandler) { return; } - dragDropContainer.current = (koenig.dragDropHandler as DragDropHandler).registerContainer( + const container = (koenig.dragDropHandler as DragDropHandler).registerContainer( containerRef, { draggableSelector, droppableSelector, isDragEnabled: enabled, @@ onDrop: _onDrop, onDropEnd: _onDropEnd } ); + + dragDropContainer.current = container; + + return () => { + container.destroy(); + if (dragDropContainer.current === container) { + dragDropContainer.current = null; + } + }; }, [ _getDraggableInfo, _getIndicatorPosition, _onDrop,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/koenig-lexical/src/hooks/useCardDragAndDrop.ts` around lines 85 - 120, The effect registers a drag container via (koenig.dragDropHandler as DragDropHandler).registerContainer and never destroys it; add a cleanup function to the React.useEffect that, when run, calls destroy() on dragDropContainer.current (if present) and clears dragDropContainer.current to null to avoid leaking listeners and duplicate callbacks; keep the existing dependencies (_getDraggableInfo, _getIndicatorPosition, _onDrop, _onDropEnd, containerRef, draggableSelector, droppableSelector, enabled, koenig.dragDropHandler, onDragEnd, onDragEnterContainer, onDragLeaveContainer, onDragStart) and ensure the cleanup runs before re-registering.packages/koenig-lexical/src/plugins/EmojiPickerPlugin.tsx (1)
170-176:⚠️ Potential issue | 🟡 MinorGuard the DOM selection before calling
getRangeAt(0).
window.getSelection()can benull, andrangeCountcan be0after blur or while the menu is settling. The non-null assertion turns that into a render-time crash.💡 Suggested fix
function getPositionStyles() { - const selectedRange = window.getSelection()!.getRangeAt(0); + const selection = window.getSelection(); + if (!selection || selection.rangeCount === 0) { + return undefined; + } + const selectedRange = selection.getRangeAt(0); const rangeRect = selectedRange.getBoundingClientRect(); return { marginTop: `${rangeRect.height}px` };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/koenig-lexical/src/plugins/EmojiPickerPlugin.tsx` around lines 170 - 176, The getPositionStyles function currently calls window.getSelection()! and getRangeAt(0) unguarded which can throw if selection is null or rangeCount is 0; update getPositionStyles to first get const sel = window.getSelection(), check sel is non-null and sel.rangeCount > 0 before calling sel.getRangeAt(0) and using its bounding rect, and return a safe fallback style (e.g., { marginTop: '0px' } or undefined) when there is no valid range so rendering cannot crash.packages/koenig-lexical/src/plugins/KoenigFocusPlugin.tsx (1)
7-16:⚠️ Potential issue | 🟠 MajorMissing cleanup return from
useEffectcauses listener leak.
editor.registerCommandreturns an unregister function that must be returned from theuseEffectto clean up when the component unmounts or dependencies change. Currently, the cleanup function is discarded.Proposed fix
useEffect(() => { - editor.registerCommand( + return editor.registerCommand( FOCUS_COMMAND, () => { onFocus?.(); return false; }, COMMAND_PRIORITY_EDITOR ); }, [editor, onFocus]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/koenig-lexical/src/plugins/KoenigFocusPlugin.tsx` around lines 7 - 16, The effect registering the focus handler is leaking because editor.registerCommand (called inside useEffect) returns an unregister function that isn't used; update the useEffect that calls editor.registerCommand with FOCUS_COMMAND and COMMAND_PRIORITY_EDITOR to capture the returned cleanup function and return it from the effect so the listener is removed when dependencies (editor, onFocus) change or the component unmounts; keep the existing call to onFocus() in the registered command.packages/koenig-lexical/src/components/ui/UrlSearchInput.tsx (1)
53-59:⚠️ Potential issue | 🟠 MajorMove event handlers from nested
<span>to<button>elements for proper keyboard and click interaction.Lines 56–57 attach click handlers to inner
<span>elements instead of their parent<button>elements. This breaks keyboard activation (Enter/Space keys) and makes clicks on button padding ineffective—only clicks on the text itself will trigger the handlers.Suggested fix
-<button className="mr-3 cursor-pointer" data-testid={`${dataTestId}-error-retry`} type="button"><span className="font-semibold underline" onClick={handleRetry}>Retry</span></button> -<button className="mr-3 cursor-pointer" data-testid={`${dataTestId}-error-pasteAsLink`} type="button"><span className="font-semibold underline" onClick={() => handlePasteAsLink?.(value)}>Paste URL as link</span></button> +<button className="mr-3 cursor-pointer" data-testid={`${dataTestId}-error-retry`} type="button" onClick={handleRetry}><span className="font-semibold underline">Retry</span></button> +<button className="mr-3 cursor-pointer" data-testid={`${dataTestId}-error-pasteAsLink`} type="button" onClick={() => handlePasteAsLink?.(value)}><span className="font-semibold underline">Paste URL as link</span></button>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/koenig-lexical/src/components/ui/UrlSearchInput.tsx` around lines 53 - 59, The click handlers for the retry and paste-as-link actions are attached to inner <span> elements which breaks keyboard activation and click area; move the onClick handlers from the inner spans to their parent <button> elements (the buttons with data-testid values `${dataTestId}-error-retry` and `${dataTestId}-error-pasteAsLink`) and remove the onClick from the spans so handleRetry and handlePasteAsLink(value) are invoked by the button elements instead; ensure the buttons keep type="button" and accessible labeling/styling (the spans may remain for styling only).packages/koenig-lexical/src/components/ui/KeyboardSelectionWithGroups.tsx (1)
41-63:⚠️ Potential issue | 🟠 MajorShort-circuit keyboard handling when there are no items.
When
items.length === 0, ArrowDown drivesselectedIndexto-1, and Enter passesundefinedtoonSelect. Unlike mouse interactions (which checkif (item.value)before updating selection), the keyboard handler lacks guards and can leave the component in an invalid state on no-results screens.Suggested fix
const handleKeydown = React.useCallback((event: KeyboardEvent) => { + if (items.length === 0) { + return; + } + if (event.key === 'ArrowDown') { event.preventDefault(); event.stopPropagation(); setSelectedIndex((i) => { return Math.min(i + 1, items.length - 1); }); setScrollSelectedIntoView(true); } if (event.key === 'ArrowUp') { event.preventDefault(); event.stopPropagation(); setSelectedIndex((i) => { return Math.max(i - 1, 0); }); setScrollSelectedIntoView(true); } if (event.key === 'Enter') { event.preventDefault(); event.stopPropagation(); - onSelect(items[selectedIndex]); + const selectedItem = items[selectedIndex]; + if (selectedItem) { + onSelect(selectedItem); + } } }, [items, selectedIndex, onSelect]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/koenig-lexical/src/components/ui/KeyboardSelectionWithGroups.tsx` around lines 41 - 63, The keyboard handler handleKeydown should short-circuit when there are no items: at the start of handleKeydown check items.length === 0 and return early so ArrowDown/ArrowUp don't drive selectedIndex to -1 and Enter doesn't call onSelect with undefined; additionally, when handling Enter ensure the selected item exists (e.g., guard that items[selectedIndex] is truthy) before calling onSelect, and keep the existing use of setSelectedIndex and setScrollSelectedIntoView for valid navigation only.packages/koenig-lexical/src/components/ui/cards/HeaderCard/v1/HeaderCard.tsx (1)
232-247:⚠️ Potential issue | 🟡 MinorMake the button-editing handlers required when
buttonis enabled.This branch renders editable controls whenever
buttonis truthy (line 233), buthandleButtonTextandhandleButtonUrlare both optional in the props interface. The code passeshandleButtonUrl!with a non-null assertion (line 247) whilehandleButtonTextis passed without assertion (line 240), masking a type contract violation. Callers can passbutton={true}without providing these handlers, leading to runtime errors on the first change event. A discriminated union onbuttonwould enforce this invariant at the type level.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/koenig-lexical/src/components/ui/cards/HeaderCard/v1/HeaderCard.tsx` around lines 232 - 247, The component renders button-editing controls when the prop button is truthy but props handleButtonText and handleButtonUrl are optional and currently asserted with handleButtonUrl!, causing possible runtime errors; change the HeaderCard props to a discriminated union on button (e.g. {button: true; handleButtonText: ..., handleButtonUrl: ...} | {button?: false; handleButtonText?: ..., handleButtonUrl?: ...}) so that when button is true the handlers are required, update usages/callers to satisfy the new type, and remove the non-null assertion on handleButtonUrl when passing handlers into InputSetting and InputUrlSetting.packages/koenig-lexical/src/components/ui/SettingsPanel.tsx (1)
34-39:⚠️ Potential issue | 🟠 MajorGuard
tabContentshape before passing toTabView.Line 38 currently treats any object-like
childrenas tab-content map. React elements/arrays are also objects, so with tabs enabled this can pass an invalid shape intoTabViewat Line 52.Proposed fix
- const tabContent = React.useMemo(() => { + const tabContent = React.useMemo<Record<string, React.ReactNode>>(() => { if (!tabs) { return {default: children}; } - return typeof children === 'object' && children !== null ? children : {default: children}; + if ( + children !== null && + typeof children === 'object' && + !React.isValidElement(children) && + !Array.isArray(children) + ) { + return children as Record<string, React.ReactNode>; + } + return {default: children}; }, [tabs, children]); ... - <TabView defaultTab={defaultTab} tabContent={tabContent as Record<string, React.ReactNode>} tabs={tabs} /> + <TabView defaultTab={defaultTab} tabContent={tabContent} tabs={tabs} />Also applies to: 52-52
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/koenig-lexical/src/components/ui/SettingsPanel.tsx` around lines 34 - 39, When building tabContent in SettingsPanel (the tabContent const used by TabView), ensure you only treat children as a tab-content map when it's a plain object (not a React element or array). Replace the current typeof children === 'object' check with a guard that verifies children !== null && typeof children === 'object' && !Array.isArray(children) && !React.isValidElement(children) before returning children; otherwise return {default: children} so TabView always receives a valid tab-content shape.
♻️ Duplicate comments (1)
packages/koenig-lexical/src/components/ui/TenorSelector.tsx (1)
132-139:⚠️ Potential issue | 🔴 Critical
highlightPrevstill missing early return afterfocusSearch().The bug flagged in a previous review remains unfixed. When
highlightedGif.index === 0, execution continues pastfocusSearch()to line 138, accessinggifs[-1]which returnsundefined.Proposed fix
function highlightPrev() { if (highlightedGif!.index === 0) { // reached the beginning, focus the search bar focusSearch(); + return; } setHighlightedGif(gifs[highlightedGif!.index - 1]); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/koenig-lexical/src/components/ui/TenorSelector.tsx` around lines 132 - 139, In highlightPrev(), when highlightedGif!.index === 0 you must return immediately after calling focusSearch() to avoid executing setHighlightedGif(gifs[highlightedGif!.index - 1]) which accesses gifs[-1]; update the function (referencing highlightPrev, highlightedGif, focusSearch, setHighlightedGif, and gifs) to add an early return right after focusSearch() so the out-of-bounds access is prevented.
🟡 Minor comments (24)
packages/koenig-lexical/demo/utils/useFileUpload.ts-49-66 (1)
49-66:⚠️ Potential issue | 🟡 MinorPotential runtime error if
typeis not a valid key.If
defaultValidatoris called whentypeis an empty string (the default) or any string not present infileTypes, line 53 will throwCannot read properties of undefined (reading 'extensions'). The early return fortype === 'file'doesn't guard against other invalid types.🛡️ Proposed defensive check
function defaultValidator(file: File): string | true { if (type === 'file') { return true; } + if (!fileTypes[type]) { + return true; // Accept all files for unknown types + } const extensions = fileTypes[type].extensions;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/koenig-lexical/demo/utils/useFileUpload.ts` around lines 49 - 66, defaultValidator currently assumes fileTypes[type] exists and reads .extensions, which will throw if type is empty or not a valid key; add a defensive guard in defaultValidator to check that type is truthy and fileTypes[type] exists (and has .extensions) before accessing it, and if the check fails either accept the file (return true) or return a clear validation message; reference the defaultValidator function and the fileTypes and type variables when making this change.packages/koenig-lexical/src/components/ui/cards/FileCard.tsx-177-181 (1)
177-181:⚠️ Potential issue | 🟡 MinorNon-null assertion on
onFileChangecould cause runtime crash.
onFileChangeis typed as optional (onFileChange?: ...) inFileCardProps, but line 180 uses a non-null assertion (onFileChange!). If theEmptyFileCardbranch renders and a user selects a file withoutonFileChangebeing provided, this will throw a runtime error.🛡️ Proposed fix
Either make
onFileChangerequired when rendering the empty state, or add a guard:<FileUploadForm fileInputRef={onFileInputRef} setFileInputRef={setFileInputRef} - onFileChange={onFileChange!} + onFileChange={onFileChange ?? (() => {})} />Alternatively, if
FileUploadFormcan accept an optional handler, remove the assertion:- onFileChange={onFileChange!} + onFileChange={onFileChange}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/koenig-lexical/src/components/ui/cards/FileCard.tsx` around lines 177 - 181, The EmptyFileCard rendering uses a non-null assertion on the optional prop onFileChange when passing it into FileUploadForm (see FileCard.tsx around the FileUploadForm usage), which can crash at runtime; fix by either requiring onFileChange when rendering the empty state or guarding the prop: update the FileCard render logic to only pass onFileChange to FileUploadForm if it is defined (or provide a no-op fallback handler) so that FileUploadForm always receives a valid function, and ensure any prop types (FileCardProps or FileUploadForm props) reflect the optional/required status consistently.packages/koenig-lexical/src/components/ui/cards/FileCard.tsx-70-81 (1)
70-81:⚠️ Potential issue | 🟡 MinorNon-null assertions on optional handlers may cause runtime errors.
The
handleFileTitle!andhandleFileDesc!assertions assume these handlers are always provided when theTextInputcomponents are rendered. While the conditional rendering (isEditing || title/desc) provides some protection, ifisEditingis true but the handlers aren't provided, this will throw at runtime.Consider adding defensive guards or making the handlers required when
isEditingis expected to be true.🛡️ Proposed fix with defensive guards
<TextInput className="h-[30px] bg-transparent text-lg font-bold leading-none tracking-tight text-black dark:text-grey-200" data-kg-file-card="fileTitle" maxLength={80} placeholder={titlePlaceholder} value={title} - onChange={handleFileTitle!} + onChange={handleFileTitle} /><TextInput className="h-[26px] bg-transparent pb-1 text-[1.6rem] font-normal leading-none text-grey-700 placeholder:text-grey-500 dark:text-grey-300 dark:placeholder:text-grey-800" data-kg-file-card="fileDescription" maxLength={100} placeholder={descPlaceholder} value={desc} - onChange={handleFileDesc!} + onChange={handleFileDesc} />This requires
TextInputto accept an optionalonChangeprop, which would be the cleaner approach.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/koenig-lexical/src/components/ui/cards/FileCard.tsx` around lines 70 - 81, The code uses non-null assertions on handleFileTitle and handleFileDesc when rendering TextInput, which can throw if isEditing is true but handlers are undefined; remove the "!" assertions and either guard the prop (e.g., only pass onChange when handleFileTitle/handleFileDesc is defined) or provide a safe no-op fallback, and update TextInput's props/type to accept an optional onChange so the component compiles cleanly; reference the handleFileTitle, handleFileDesc, TextInput, isEditing, title and desc symbols when making the changes.packages/koenig-lexical/src/components/ui/TenorSelector.tsx-359-359 (1)
359-359:⚠️ Potential issue | 🟡 MinorUnsafe cast of
errortostring.The
errorprop is typed asunknownbut cast directly tostring. If error is anErrorobject or other type, the Error component may not render it correctly.Proposed fix: safely convert error to string
- {!!error && <div data-testid="tenor-selector-error"><Error error={error as string} /></div>} + {!!error && <div data-testid="tenor-selector-error"><Error error={error instanceof Error ? error.message : String(error)} /></div>}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/koenig-lexical/src/components/ui/TenorSelector.tsx` at line 359, The line in TenorSelector.tsx unsafely casts the unknown error to string; replace that cast by normalizing error into a string before passing to the Error component (e.g., check typeof error === 'string', else if error instanceof Error use error.message, otherwise use String(error or 'Unknown error')). Update the JSX around {!!error && <div data-testid="tenor-selector-error"><Error error={...} /></div>} to pass the safely converted string instead of (error as string).packages/koenig-lexical/src/nodes/EmailCtaNode.tsx-83-93 (1)
83-93:⚠️ Potential issue | 🟡 MinorUse public getter for
segmentproperty to match other properties.Line 90 accesses
this.__segmentdirectly while other base properties use public getters (this.alignment,this.buttonText,this.buttonUrl,this.showButton,this.showDividers). Change tothis.segmentfor consistency.Diff
<EmailCtaNodeComponent alignment={this.alignment as string} buttonText={this.buttonText as string} buttonUrl={this.buttonUrl as string} htmlEditor={this.__htmlEditor} htmlEditorInitialState={this.__htmlEditorInitialState as string | undefined} nodeKey={this.getKey()} - segment={this.__segment as string} + segment={this.segment as string} showButton={this.showButton as boolean} showDividers={this.showDividers as boolean} />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/koenig-lexical/src/nodes/EmailCtaNode.tsx` around lines 83 - 93, The code is directly accessing the private field this.__segment when rendering EmailCtaNodeComponent; change it to use the public getter this.segment (matching how alignment, buttonText, buttonUrl, showButton, and showDividers are used) so replace the this.__segment prop passed to EmailCtaNodeComponent with this.segment to keep property access consistent in the EmailCtaNode render method.packages/koenig-lexical/src/nodes/FileNodeComponent.tsx-64-74 (1)
64-74:⚠️ Potential issue | 🟡 MinorNon-null assertion on
filescould cause runtime error.
e.target.filescan benull(e.g., if the user cancels the file dialog). Thefiles!assertion on line 74 bypasses this check. Consider adding an early return guard.Proposed fix
const onFileChange = async (e: React.ChangeEvent<HTMLInputElement>) => { const files = e.target.files; + if (!files || files.length === 0) { + return; + } // reset original src so it can be replaced with preview and upload progress editor.update(() => { const node = $getNodeByKey(nodeKey) as FileNode | null; if (!node) {return;} node.src = ''; }); - return await fileUploadHandler(Array.from(files!), nodeKey, editor, uploader.upload); + return await fileUploadHandler(Array.from(files), nodeKey, editor, uploader.upload); };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/koenig-lexical/src/nodes/FileNodeComponent.tsx` around lines 64 - 74, The handler onFileChange uses e.target.files with a non-null assertion (files!) which can throw if the user cancels the dialog; add an early guard to check if const files = e.target.files is null or has length 0 and return early before calling fileUploadHandler, then pass Array.from(files) without the non-null assertion; keep the existing editor.update block and use nodeKey, editor and uploader.upload as before.packages/koenig-lexical/demo/utils/fetchEmbed.ts-3-6 (1)
3-6:⚠️ Potential issue | 🟡 MinorDead code:
new URL()throws on invalid input, never returns falsy.The check
if (!urlObject)is unreachable becausenew URL(url)either succeeds and returns a validURLobject, or throws aTypeErrorfor invalid URLs. The error should be caught in the existing try-catch instead.Suggested fix
export async function fetchEmbed(url: string, {type}: {type: string}) { console.log('fetchEmbed', {url, type}); - const urlObject = new URL(url); - if (!urlObject) { - throw new Error('No URL specified.'); - } await delay(process.env.NODE_ENV === 'test' ? 50 : 1500); try { + const urlObject = new URL(url); if (type === 'bookmark') {Or wrap URL parsing in try-catch if you want a custom error message.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/koenig-lexical/demo/utils/fetchEmbed.ts` around lines 3 - 6, The null-check after constructing the URL is dead code because new URL(url) throws on invalid input; remove the unreachable if (!urlObject) branch in fetchEmbed (or the function containing const urlObject = new URL(url)) and rely on the existing try-catch to handle invalid URLs, or explicitly wrap the URL construction in its own try-catch to throw a custom error message before proceeding with the rest of the function.packages/koenig-lexical/src/components/ui/TabView.tsx-14-15 (1)
14-15:⚠️ Potential issue | 🟡 MinorGuard the initial tab selection.
tabs[0].idwill throw whentabsis empty, and an unknowndefaultTableaves the panel with no matching content. Validate the initial id before seeding state and bail out gracefully when there are no tabs.💡 Suggested fix
const TabView = ({tabs, defaultTab, tabContent}: TabViewProps) => { - const [activeTab, setActiveTab] = useState(defaultTab || tabs[0].id); + const initialTab = tabs.find(tab => tab.id === defaultTab)?.id ?? tabs[0]?.id; + + if (!initialTab) { + return null; + } + + const [activeTab, setActiveTab] = useState(initialTab);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/koenig-lexical/src/components/ui/TabView.tsx` around lines 14 - 15, The TabView component seeds its state with defaultTab or tabs[0].id which crashes when tabs is empty or selects a defaultTab that doesn't exist; update TabView (props TabViewProps, variables activeTab and setActiveTab, and rendering that uses tabContent) to first validate tabs is non-empty and that defaultTab exists in tabs (e.g., find by id), initialize activeTab to a safe value (the validated defaultTab id or the first tab's id) and if no tabs exist return early/empty UI to bail out gracefully so the panel never tries to render missing content.packages/koenig-lexical/src/nodes/header/v2/HeaderNodeComponent.tsx-143-152 (1)
143-152:⚠️ Potential issue | 🟡 MinorValidate
filesbefore clearing the current image.This handler accepts
null, butArray.from(files!)still assumes a value is present. If a caller passesnullor an empty selection here, the current background image is cleared before the upload path fails.💡 Suggested fix
const handleImageChange = async (files: File[] | FileList | null) => { + if (!files || files.length === 0) { + return; + } + // reset original src so it can be replaced with preview and upload progress editor.update(() => { const node = $getHeaderNodeByKey(nodeKey); if (!node) {return;} node.backgroundImageSrc = ''; }); - const result = await backgroundImageUploadHandler(Array.from(files!), imageUploader.upload); + const result = await backgroundImageUploadHandler(Array.from(files), imageUploader.upload);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/koenig-lexical/src/nodes/header/v2/HeaderNodeComponent.tsx` around lines 143 - 152, The handler handleImageChange clears the header node's backgroundImageSrc before validating files and may wipe the image when files is null/empty; first validate that files is non-null and has at least one file (and/or FileList.length > 0) and return early if not, then proceed to call backgroundImageUploadHandler and only update node.backgroundImageSrc/preview/upload progress after a successful files array conversion and upload start; locate the logic in HeaderNodeComponent's handleImageChange and move the editor.update that clears backgroundImageSrc to after the files checks (or guard it with a check on files.length) so the image is not cleared on null/empty input.packages/koenig-lexical/src/nodes/ImageNodeComponent.tsx-166-177 (1)
166-177:⚠️ Potential issue | 🟡 MinorBail out before blanking the node when no files were selected.
e.target.filesis nullable here, but the code clearsnode.srcbefore passingfiles!into the upload handler. A null or empty selection path will leave the current image blank with no replacement.💡 Suggested fix
const onFileChange = async (e: React.ChangeEvent<HTMLInputElement>) => { const files = e.target.files; + + if (!files || files.length === 0) { + return; + } // reset original src so it can be replaced with preview and upload progress editor.update(() => { const node = $getImageNodeByKey(nodeKey); if (!node) {return;} if (!node) {return;} node.src = ''; }); - return await imageUploadHandler(files!, nodeKey, editor, imageUploader.upload); + return await imageUploadHandler(files, nodeKey, editor, imageUploader.upload); };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/koenig-lexical/src/nodes/ImageNodeComponent.tsx` around lines 166 - 177, The onFileChange handler clears node.src before verifying files exist, which blanks the image when no file was selected; modify onFileChange to check e.target.files (and files.length) and return early if null/empty before calling editor.update or blanking the node, then only call imageUploadHandler(files, nodeKey, editor, imageUploader.upload) when files are present; reference the onFileChange function, $getImageNodeByKey, node.src, imageUploadHandler and imageUploader.upload to locate where to add the early-return guard.packages/koenig-lexical/src/components/ui/cards/CodeBlockCard.tsx-197-213 (1)
197-213:⚠️ Potential issue | 🟡 MinorHide the language input semantically, not just visually.
When
showLanguageis false the field only getsopacity-0, so it can still take focus and intercept clicks in the editor’s top-right corner. Making the wrapperrelativeand disabling pointer/tab interaction when hidden avoids that.💡 Suggested fix
- <div className="not-kg-prose min-h-[170px]"> + <div className="not-kg-prose relative min-h-[170px]"> <CodeMirror autoFocus={true} // autofocus the editor whenever it is rendered basicSetup={false} // basic setup includes unnecessary extensions extensions={extensions} value={code} @@ <input + aria-hidden={!showLanguage} aria-label="Code card language" - className={`z-999 absolute right-1.5 top-1.5 w-1/5 rounded-md border border-grey-300 px-2 py-1 font-sans text-[1.3rem] leading-4 text-grey-900 transition-opacity focus-visible:outline-none dark:border-grey-900 dark:text-grey-400 ${showLanguage ? 'opacity-100' : 'opacity-0'}`} + className={`z-999 absolute right-1.5 top-1.5 w-1/5 rounded-md border border-grey-300 px-2 py-1 font-sans text-[1.3rem] leading-4 text-grey-900 transition-opacity focus-visible:outline-none dark:border-grey-900 dark:text-grey-400 ${showLanguage ? 'opacity-100' : 'pointer-events-none opacity-0'}`} data-testid="code-card-language" placeholder="Language..." + tabIndex={showLanguage ? 0 : -1} type="text" value={language} onChange={onLanguageChange} />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/koenig-lexical/src/components/ui/cards/CodeBlockCard.tsx` around lines 197 - 213, The language input is only visually hidden via opacity, so when showLanguage is false it still receives focus and intercepts clicks; update the CodeBlockCard component so the wrapper div (the element with className "not-kg-prose min-h-[170px]") is positioned relative and the input element (data-testid="code-card-language") is made non-interactive when hidden: add conditional classes/attributes such that when showLanguage is false the input gets pointer-events-none (or disabled), tabIndex={-1}, and aria-hidden="true" (and when true restore tabIndex=0 and remove aria-hidden), while keeping the existing visual opacity logic to preserve appearance. This targets the CodeBlockCard input and surrounding div to prevent the input from stealing focus/clicks when hidden.packages/koenig-lexical/src/components/ui/KeyboardSelection.tsx-39-43 (1)
39-43:⚠️ Potential issue | 🟡 MinorPotential
undefinedpassed toonSelectwhen items is empty.If
itemsis an empty array,items[selectedIndex]will beundefined, butonSelectexpectsunknown(implying a valid item). Consider guarding against empty arrays.Proposed fix
if (event.key === 'Enter') { event.preventDefault(); event.stopPropagation(); - onSelect(items[selectedIndex]); + if (items.length > 0) { + onSelect(items[selectedIndex]); + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/koenig-lexical/src/components/ui/KeyboardSelection.tsx` around lines 39 - 43, The Enter key handler in KeyboardSelection.tsx may call onSelect(items[selectedIndex]) when items is empty or selectedIndex is out of range; update the handler in the component (the block that checks if (event.key === 'Enter')) to first verify items.length > 0 and that selectedIndex is a valid index (e.g., selectedIndex >= 0 && selectedIndex < items.length) before calling onSelect, and bail out (or optionally call a safe fallback) if the check fails so undefined is never passed to onSelect.packages/koenig-lexical/demo/RestrictedContentDemo.tsx-89-89 (1)
89-89:⚠️ Potential issue | 🟡 MinorNon-null assertion on ref may cause runtime error.
containerRef.current!assumes the ref is always attached, but if this code runs before the ref is set or after unmount, it will throw. Consider adding a null check.Proposed fix
//scroll to the bottom of the container - containerRef.current!.scrollTop = containerRef.current!.scrollHeight; + if (containerRef.current) { + containerRef.current.scrollTop = containerRef.current.scrollHeight; + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/koenig-lexical/demo/RestrictedContentDemo.tsx` at line 89, The current assignment uses a non-null assertion on containerRef.current (in RestrictedContentDemo) which can throw if the ref isn't attached; update the code that sets containerRef.current!.scrollTop = containerRef.current!.scrollHeight to first guard that containerRef.current exists (e.g., retrieve into a local const el = containerRef.current and return or skip if el is null) and only then set el.scrollTop = el.scrollHeight; this ensures safe access to scrollTop/scrollHeight and avoids runtime errors when the ref is undefined or after unmount.packages/koenig-lexical/src/components/ui/FloatingLinkToolbar.tsx-83-84 (1)
83-84:⚠️ Potential issue | 🟡 MinorSelection set even when range is invalid.
$setSelection(selection)andonEditLink({href})are called unconditionally after theifblock. IffirstChild/lastChildare null or not TextNodes, an empty/invalid selection is set andonEditLinkis still invoked, which may cause unexpected behavior.Proposed fix to only proceed when selection is valid
const firstChild = linkNode.getFirstChild(); const lastChild = linkNode.getLastChild(); - if (firstChild && lastChild) { + if (firstChild && lastChild && $isTextNode(firstChild) && $isTextNode(lastChild)) { selection.setTextNodeRange( firstChild, 0, lastChild, lastChild.getTextContentSize() ); + $setSelection(selection); + onEditLink({href}); } - $setSelection(selection); - onEditLink({href}); }); };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/koenig-lexical/src/components/ui/FloatingLinkToolbar.tsx` around lines 83 - 84, The code currently calls $setSelection(selection) and onEditLink({href}) unconditionally which can set an invalid selection when firstChild/lastChild are null or not TextNodes; update the logic in FloatingLinkToolbar so you only call $setSelection(selection) and onEditLink({href}) when the computed selection is valid (i.e., firstChild and lastChild exist and are TextNodes and the selection range is non-empty/valid). Concretely, add a guard around the existing calls (or move them inside the existing if block that verifies firstChild/lastChild), referencing the existing variables/operations: firstChild, lastChild, selection, $setSelection, and onEditLink to ensure those functions are invoked only after validation.packages/koenig-lexical/src/components/ui/FloatingLinkToolbar.tsx-73-82 (1)
73-82:⚠️ Potential issue | 🟡 MinorAdd type guards for
setTextNodeRangearguments.
linkNode.getFirstChild()andgetLastChild()returnLexicalNode | null, butsetTextNodeRangeexpectsTextNodearguments. If a link contains non-text content, this will cause a runtime error. Add$isTextNodeguards before callingsetTextNodeRange:Proposed fix
+import {$isTextNode} from 'lexical'; ... const firstChild = linkNode.getFirstChild(); const lastChild = linkNode.getLastChild(); - if (firstChild && lastChild) { + if (firstChild && lastChild && $isTextNode(firstChild) && $isTextNode(lastChild)) { selection.setTextNodeRange( firstChild, 0, lastChild, lastChild.getTextContentSize() ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/koenig-lexical/src/components/ui/FloatingLinkToolbar.tsx` around lines 73 - 82, The code calls selection.setTextNodeRange using linkNode.getFirstChild() and getLastChild() which return LexicalNode|null but setTextNodeRange requires TextNode; add type guards using $isTextNode to ensure both firstChild and lastChild are TextNode before calling selection.setTextNodeRange (i.e., check $isTextNode(firstChild) && $isTextNode(lastChild) and only then pass them to selection.setTextNodeRange), so non-text children inside the link won't cause a runtime error.packages/koenig-lexical/src/components/ui/cards/SignupCard.tsx-520-520 (1)
520-520:⚠️ Potential issue | 🟡 MinorAdditional non-null assertions on optional props.
Similar to line 295,
onFileChange!(line 520) andhandleLabels!(line 567) use non-null assertions on optional props. These should either be made required or guarded.Also applies to: 567-567
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/koenig-lexical/src/components/ui/cards/SignupCard.tsx` at line 520, The code is using non-null assertions on optional props (onFileChange! and handleLabels!) in SignupCard; either make these props required in the SignupCardProps/type or guard their use. Fix by updating the prop type to mark onFileChange and handleLabels as required, or remove the "!" and add runtime guards where used (e.g., only pass onFileChange to the child or call handleLabels when it's defined). Reference the SignupCard component and the prop names onFileChange and handleLabels to locate and update the prop interface and all usages accordingly.packages/koenig-lexical/src/components/ui/cards/EmbedCard.tsx-117-127 (1)
117-127:⚠️ Potential issue | 🟡 MinorNon-null assertions in
handleLoadmay fail for cross-origin iframes.
iframeRef.current!.contentDocument!.bodyandiframeRef.current!.contentWindow!.documentassume the iframe's content is accessible. For cross-origin content (e.g., some embeds),contentDocumentwill benulldue to same-origin policy, causing a runtime error.🛡️ Proposed defensive guards
const handleLoad = () => { - const iframeBody = iframeRef.current!.contentDocument!.body; + const iframeBody = iframeRef.current?.contentDocument?.body; + if (!iframeBody) { + return; + } // apply styles iframeBody.style.display = 'flex'; iframeBody.style.margin = '0'; iframeBody.style.justifyContent = 'center'; // resize first load handleResize(); // start listening to mutations when the iframe content is loaded - mutationObserver.observe(iframeRef.current!.contentWindow!.document, config); + const contentDoc = iframeRef.current?.contentWindow?.document; + if (contentDoc) { + mutationObserver.observe(contentDoc, config); + } };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/koenig-lexical/src/components/ui/cards/EmbedCard.tsx` around lines 117 - 127, The handleLoad function uses non-null assertions on iframeRef.current!.contentDocument!.body and iframeRef.current!.contentWindow!.document which will throw for cross-origin embeds; update handleLoad to defensively check iframeRef.current exists and that iframeRef.current.contentDocument and iframeRef.current.contentWindow are non-null and accessible before touching body styles or calling mutationObserver.observe, and only call handleResize and start observing when those checks pass (otherwise skip styling/observing for cross-origin iframes); replace uses of non-null assertions with these safe guards and bail out early if the iframe is cross-origin.packages/koenig-lexical/src/components/ui/cards/SignupCard.tsx-295-295 (1)
295-295:⚠️ Potential issue | 🟡 MinorNon-null assertion on optional
onFileChangeprop.
onFileChangeis declared as optional (onFileChange?: ...) inSignupCardProps, but it's used with a non-null assertion (onFileChange!). If the parent component doesn't passonFileChange, this will throw a runtime error.Either make
onFileChangerequired in the interface, or guard theMediaUploaderrendering with a check foronFileChange.🛡️ Option 1: Make prop required
interface SignupCardProps { // ... - onFileChange?: (e: React.ChangeEvent<HTMLInputElement>) => void; + onFileChange: (e: React.ChangeEvent<HTMLInputElement>) => void; // ... }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/koenig-lexical/src/components/ui/cards/SignupCard.tsx` at line 295, SignupCard uses the optional prop onFileChange with a non-null assertion (onFileChange!) when rendering MediaUploader, which can crash if the parent omits it; update either the SignupCardProps to make onFileChange required or (preferred) guard the MediaUploader render/prop by checking if onFileChange is defined before passing it (e.g., only render MediaUploader or pass the prop when onFileChange !== undefined) so that SignupCard and MediaUploader never receive a forced-null value.packages/koenig-lexical/src/nodes/BookmarkNodeComponent.tsx-146-149 (1)
146-149:⚠️ Potential issue | 🟡 MinorDuplicate null guard should be removed.
Same issue as above — lines 148-149 contain duplicate
if (!node) {return;}statements with inconsistent indentation on line 148.🧹 Proposed fix
editor.update(() => { const node = $getBookmarkNodeByKey(nodeKey); - if (!node) {return;} if (!node) {return;} node.url = response.url;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/koenig-lexical/src/nodes/BookmarkNodeComponent.tsx` around lines 146 - 149, In BookmarkNodeComponent.tsx inside the editor.update callback where you call $getBookmarkNodeByKey(nodeKey), remove the duplicate null-check so only a single if (!node) { return; } remains (currently duplicated on two adjacent lines) and correct the indentation to match surrounding code; locate the checks near the call to $getBookmarkNodeByKey and keep one properly indented guard before using the node.packages/koenig-lexical/src/components/KoenigComposableEditor.tsx-135-135 (1)
135-135:⚠️ Potential issue | 🟡 MinorRemove empty JSX expression.
The
{ }expression on line 135 appears to be a leftover from refactoring and serves no purpose.🧹 Proposed fix
- { } {floatingAnchorElem && (<FloatingToolbarPlugin anchorElem={floatingAnchorElem} hiddenFormats={hiddenFormats} isSnippetsEnabled={isSnippetsEnabled} />)}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/koenig-lexical/src/components/KoenigComposableEditor.tsx` at line 135, Remove the stray empty JSX expression "{ }" inside the KoenigComposableEditor component JSX (file: KoenigComposableEditor.tsx) which is a no-op leftover from refactoring; locate the JSX render/return block inside the KoenigComposableEditor function/class and delete the empty expression so the JSX tree contains only meaningful nodes and no empty expressions.packages/koenig-lexical/src/components/ui/InputList.tsx-120-123 (1)
120-123:⚠️ Potential issue | 🟡 MinorType mismatch:
onChangecalled with extra argument.When
onSelectis undefined,onChangeis called withtypedItem.typeas a second argument, butonChangeis typed as(value: string) => void. While JavaScript ignores extra arguments, this indicates a type design issue.🔧 Proposed fix
const onSelectEvent = (item: unknown) => { const typedItem = item as {value: string; type?: string}; - (onSelect || onChange)(typedItem.value, typedItem.type); + if (onSelect) { + onSelect(typedItem.value, typedItem.type); + } else { + onChange(typedItem.value); + } };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/koenig-lexical/src/components/ui/InputList.tsx` around lines 120 - 123, onSelectEvent currently calls (onSelect || onChange)(typedItem.value, typedItem.type) which passes a second arg to onChange even though onChange is typed as (value: string) => void; change the call so it respects each handler's signature: if onSelect is defined call onSelect(typedItem.value, typedItem.type), otherwise call onChange(typedItem.value). Update the onSelectEvent implementation to explicitly branch on onSelect rather than using the || shorthand to avoid the type mismatch (referencing onSelectEvent, onSelect, onChange, and typedItem).packages/koenig-lexical/src/nodes/BookmarkNodeComponent.tsx-120-123 (1)
120-123:⚠️ Potential issue | 🟡 MinorDuplicate null guard should be removed.
Lines 122-123 contain two identical
if (!node) {return;}statements. The second check is redundant and appears to be a copy-paste error. Additionally, line 122 has inconsistent indentation (3 spaces instead of 4).🧹 Proposed fix
editor.update(() => { const node = $getBookmarkNodeByKey(nodeKey); - if (!node) {return;} if (!node) {return;} node.url = href;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/koenig-lexical/src/nodes/BookmarkNodeComponent.tsx` around lines 120 - 123, Remove the duplicate null guard inside the editor.update callback: keep a single check after obtaining const node = $getBookmarkNodeByKey(nodeKey) and delete the redundant if (!node) { return; } that follows it; also correct the indentation of the remaining guard to match surrounding code (use 4 spaces). This change affects the editor.update(...) callback in BookmarkNodeComponent.tsx where $getBookmarkNodeByKey and node are used.packages/koenig-lexical/src/components/KoenigComposableEditor.tsx-87-88 (1)
87-88:⚠️ Potential issue | 🟡 MinorType mismatch: passing serialized JSON where EditorState is expected.
Line 88 passes
json(a serialized JSON object from.toJSON()) tosharedOnChange, which expects anEditorStateinstance according to theSharedOnChangeContextcallback signature. The castas unknown as import('lexical').EditorStateworks around the type mismatch but is semantically incorrect and misleading. The callback signature should accept the actual JSON type being passed.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/koenig-lexical/src/components/KoenigComposableEditor.tsx` around lines 87 - 88, The code is serializing primaryEditorState via primaryEditorState.toJSON() and casting that JSON to import('lexical').EditorState when calling sharedOnChange, which is a type mismatch; either pass the real EditorState instance to sharedOnChange or change the SharedOnChangeContext callback type to accept the serialized shape. Fix by updating the sharedOnChange/SharedOnChangeContext signature to accept the JSON type returned by primaryEditorState.toJSON() (e.g., the concrete serialized editor state type or a Record<string, unknown>/SerializedEditorState alias) and adjust all call sites (including the call in KoenigComposableEditor where you currently call sharedOnChange(json, editor, new Set<string>())) to use that new serialized type, or alternatively stop calling .toJSON() and pass the actual EditorState object into sharedOnChange so the existing EditorState type remains correct.packages/koenig-lexical/src/components/ui/SettingsPanel.tsx-373-377 (1)
373-377:⚠️ Potential issue | 🟡 Minor
dataTestIdexists in props but is not wired inMediaUploadSetting.
MediaUploadSettingPropsincludesdataTestId(Line 376), but Line 400 does not destructure it and the container test id is hardcoded. This makes the prop ineffective.Proposed fix
-export function MediaUploadSetting({className, imgClassName, label, hideLabel, onFileChange, isDraggedOver, placeholderRef, src, alt, isLoading, errors = [], progress, onRemoveMedia, icon, desc, size, type, stacked, borderStyle, mimeTypes, isPinturaEnabled, openImageEditor, setFileInputRef}: MediaUploadSettingProps) { +export function MediaUploadSetting({className, imgClassName, dataTestId, label, hideLabel, onFileChange, isDraggedOver, placeholderRef, src, alt, isLoading, errors = [], progress, onRemoveMedia, icon, desc, size, type, stacked, borderStyle, mimeTypes, isPinturaEnabled, openImageEditor, setFileInputRef}: MediaUploadSettingProps) { return ( - <div className={clsx(className, !stacked && 'flex justify-between gap-3')} data-testid="media-upload-setting"> + <div className={clsx(className, !stacked && 'flex justify-between gap-3')} data-testid={dataTestId ?? 'media-upload-setting'}>Also applies to: 400-403
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/koenig-lexical/src/components/ui/SettingsPanel.tsx` around lines 373 - 377, The MediaUploadSetting component doesn't use the dataTestId prop declared in MediaUploadSettingProps; update the MediaUploadSetting function signature to destructure dataTestId (alongside className, imgClassName, label) and replace the hardcoded data-testid on the container element with the passed-in dataTestId (optionally keep a fallback string if needed) so the prop is actually wired and controls the test id.
| const getItem = (item: unknown, selected: boolean, onMouseOver: () => void, scrollIntoView: boolean) => { | ||
| const typedItem = item as {value: string; label: string; highlight?: boolean; Icon?: React.ComponentType; MetaIcon?: React.ComponentType; metaText?: string; metaIconTitle?: string}; | ||
| return ( | ||
| <LinkInputSearchItem | ||
| key={item.value} | ||
| key={typedItem.value} | ||
| dataTestId={testId} | ||
| highlightString={query} | ||
| item={item} | ||
| item={typedItem} | ||
| scrollIntoView={scrollIntoView} | ||
| selected={selected} | ||
| onClick={onSelect} | ||
| onClick={onSelect as () => void} | ||
| onMouseOver={onMouseOver} | ||
| /> | ||
| ); | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Verify LinkInputSearchItem's onClick signature to confirm the fix
ast-grep --pattern 'function LinkInputSearchItem($$$) {
$$$
}'
rg -n 'onClick' packages/koenig-lexical/src/components/ui/LinkInputSearchItem.tsx -A2 -B2Repository: TryGhost/Koenig
Length of output: 3825
🏁 Script executed:
#!/bin/bash
# Find InputListItem definition and see how it handles onClick
rg -n 'InputListItem' packages/koenig-lexical/src/components/ui/ -l
fd -t f 'InputListItem' packages/koenig-lexical/src/
# Search for InputListItem interface/component definition
rg -B5 -A15 'interface InputListItemProps|type InputListItemProps|export.*InputListItem' packages/koenig-lexical/src/ -g '*.tsx' -g '*.ts'Repository: TryGhost/Koenig
Length of output: 3234
🏁 Script executed:
#!/bin/bash
# Search for all uses of LinkInputSearchItem to understand if the () => void signature is intentional
rg -n 'LinkInputSearchItem' packages/koenig-lexical/src/ -g '*.tsx' -g '*.ts' --type-add 'tsx:*.tsx' --type-add 'ts:*.ts'
# Also check the LinkInputSearchItem props definition specifically
rg -B2 -A8 'interface LinkInputSearchItemProps' packages/koenig-lexical/src/components/ui/LinkInputSearchItem.tsxRepository: TryGhost/Koenig
Length of output: 1424
🏁 Script executed:
#!/bin/bash
# Check how UrlSearchInput uses LinkInputSearchItem
rg -B5 -A10 '<LinkInputSearchItem' packages/koenig-lexical/src/components/ui/UrlSearchInput.tsx
# Check how LinkInputWithSearch uses LinkInputSearchItem
rg -B5 -A10 '<LinkInputSearchItem' packages/koenig-lexical/src/components/ui/LinkInputWithSearch.tsxRepository: TryGhost/Koenig
Length of output: 1228
🏁 Script executed:
#!/bin/bash
# Get the full LinkInputSearchItem interface definition
rg -B2 -A15 'interface LinkInputSearchItemProps' packages/koenig-lexical/src/components/ui/LinkInputSearchItem.tsx -A 20Repository: TryGhost/Koenig
Length of output: 792
Fix onClick handler to properly receive the item parameter.
LinkInputSearchItemProps incorrectly defines onClick?: () => void while InputListItem (which it wraps) expects onClick?: (item: unknown) => void and calls it with the item. The cast onSelect as () => void masks this type mismatch. The proposed fix using onClick={() => onSelect(typedItem)} correctly captures and passes the item to the handler.
Note: This type mismatch affects all three callers of LinkInputSearchItem (AtLinkResultsPopup, UrlSearchInput, LinkInputWithSearch), suggesting the root cause is LinkInputSearchItemProps.onClick being incorrectly typed. Ideally, the prop type should be fixed to onClick?: (item: unknown) => void to resolve this systematically across all usage sites.
🐛 Proposed fix
return (
<LinkInputSearchItem
key={typedItem.value}
dataTestId={testId}
highlightString={query}
item={typedItem}
scrollIntoView={scrollIntoView}
selected={selected}
- onClick={onSelect as () => void}
+ onClick={() => onSelect(typedItem)}
onMouseOver={onMouseOver}
/>
);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const getItem = (item: unknown, selected: boolean, onMouseOver: () => void, scrollIntoView: boolean) => { | |
| const typedItem = item as {value: string; label: string; highlight?: boolean; Icon?: React.ComponentType; MetaIcon?: React.ComponentType; metaText?: string; metaIconTitle?: string}; | |
| return ( | |
| <LinkInputSearchItem | |
| key={item.value} | |
| key={typedItem.value} | |
| dataTestId={testId} | |
| highlightString={query} | |
| item={item} | |
| item={typedItem} | |
| scrollIntoView={scrollIntoView} | |
| selected={selected} | |
| onClick={onSelect} | |
| onClick={onSelect as () => void} | |
| onMouseOver={onMouseOver} | |
| /> | |
| ); | |
| }; | |
| const getItem = (item: unknown, selected: boolean, onMouseOver: () => void, scrollIntoView: boolean) => { | |
| const typedItem = item as {value: string; label: string; highlight?: boolean; Icon?: React.ComponentType; MetaIcon?: React.ComponentType; metaText?: string; metaIconTitle?: string}; | |
| return ( | |
| <LinkInputSearchItem | |
| key={typedItem.value} | |
| dataTestId={testId} | |
| highlightString={query} | |
| item={typedItem} | |
| scrollIntoView={scrollIntoView} | |
| selected={selected} | |
| onClick={() => onSelect(typedItem)} | |
| onMouseOver={onMouseOver} | |
| /> | |
| ); | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/koenig-lexical/src/components/ui/AtLinkResultsPopup.tsx` around
lines 110 - 124, The onClick handler is currently mismatched:
LinkInputSearchItemProps defines onClick?: () => void but InputListItem and
callers expect onClick?: (item: unknown) => void, and getItem in
AtLinkResultsPopup casts onSelect to () => void; fix by updating the prop type
on LinkInputSearchItemProps to onClick?: (item: unknown) => void (so
UrlSearchInput, LinkInputWithSearch and AtLinkResultsPopup align) and update
getItem (in AtLinkResultsPopup) to pass the selected item into the handler
(e.g., call onSelect(typedItem) via an arrow) instead of casting onSelect;
ensure LinkInputSearchItem and its uses accept and forward the item parameter
correctly.
| interface ColorPickerSettingProps { | ||
| label: string; | ||
| isExpanded?: boolean; | ||
| onSwatchChange: (value: string) => void; | ||
| onPickerChange: (value: string) => void; | ||
| onTogglePicker: (expanded: boolean) => void; | ||
| value?: string; | ||
| swatches: {hex?: string; accent?: boolean; transparent?: boolean; image?: boolean; title: string; customContent?: React.ReactNode}[]; | ||
| eyedropper?: boolean; | ||
| hasTransparentOption?: boolean; | ||
| dataTestId?: string; | ||
| children?: React.ReactNode; | ||
| showChildren?: boolean; | ||
| } |
There was a problem hiding this comment.
showChildren is declared but ignored in ColorPickerSetting.
Line 341 exposes showChildren, but the component never reads it and always renders children (Line 365). That’s an API/behavior mismatch and can regress callers relying on hidden children state.
Proposed fix
-export function ColorPickerSetting({label, isExpanded, onSwatchChange, onPickerChange, onTogglePicker, value, swatches, eyedropper, hasTransparentOption, dataTestId, children}: ColorPickerSettingProps) {
+export function ColorPickerSetting({label, isExpanded, onSwatchChange, onPickerChange, onTogglePicker, value, swatches, eyedropper, hasTransparentOption, dataTestId, children, showChildren = true}: ColorPickerSettingProps) {
...
- {children}
+ {showChildren ? children : null}
</ColorIndicator>Also applies to: 344-366
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/koenig-lexical/src/components/ui/SettingsPanel.tsx` around lines 329
- 342, The prop showChildren on ColorPickerSettingProps is declared but never
used by the ColorPickerSetting component; update the ColorPickerSetting
component to conditionally render the children based on the showChildren prop
(falling back to current behavior when showChildren is undefined), i.e., change
the unconditional children render to something like: if showChildren is true
render children, if false do not render, and if undefined preserve existing
behavior; ensure you reference the prop name showChildren in the component’s
parameter destructuring and use it where children are rendered so callers can
control visibility.
| setupNestedEditor(this, '__callToActionHtmlEditor', {editor: dataset.callToActionHtmlEditor as LexicalEditor | undefined, nodes: BASIC_NODES}); | ||
| setupNestedEditor(this, '__sponsorLabelHtmlEditor', {editor: dataset.sponsorLabelHtmlEditor as LexicalEditor | undefined, nodes: BASIC_NODES}); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Inspect setupNestedEditor definition and expected editor shape
rg -n -C3 --type=ts --type=tsx '\bsetupNestedEditor\b'
# 2) Find all places where unknown values are asserted to LexicalEditor
rg -n -C2 --type=ts --type=tsx 'as LexicalEditor \| undefined'
# 3) Trace CallToActionNode creation paths and dataset origins
rg -n -C3 --type=ts --type=tsx '\$createCallToActionNode\(|new CallToActionNode\(|INSERT_CALL_TO_ACTION_COMMAND'Repository: TryGhost/Koenig
Length of output: 86
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Inspect setupNestedEditor definition and expected editor shape
rg -n -C3 'setupNestedEditor' --glob '*.ts' --glob '*.tsx'
# 2) Find all places where unknown values are asserted to LexicalEditor
rg -n -C2 'as LexicalEditor' --glob '*.ts' --glob '*.tsx'
# 3) Trace CallToActionNode creation paths and dataset origins
rg -n -C3 '\$createCallToActionNode|new CallToActionNode|INSERT_CALL_TO_ACTION_COMMAND' --glob '*.ts' --glob '*.tsx'Repository: TryGhost/Koenig
Length of output: 35075
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check CallToActionPlugin to see what calls $createCallToActionNode
cat -n packages/koenig-lexical/src/plugins/CallToActionPlugin.tsx | head -30
# Check if there's any validation in CallToActionNode constructor before setupNestedEditor
cat -n packages/koenig-lexical/src/nodes/CallToActionNode.tsx | head -80
# Check the SetupNestedEditorOptions type definition
rg -n -B5 -A10 'interface SetupNestedEditorOptions' --glob '*.ts' --glob '*.tsx'
# Check if dataset is validated/typed anywhere
rg -n 'createCallToActionNode\|INSERT_CALL_TO_ACTION_COMMAND' --glob '*.ts' --glob '*.tsx' | head -20Repository: TryGhost/Koenig
Length of output: 6662
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Get the full setupNestedEditor implementation
cat -n packages/koenig-lexical/src/utils/nested-editors.ts
# Also check populateNestedEditor which is called on line 65-68 of CallToActionNodeRepository: TryGhost/Koenig
Length of output: 2661
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check how CallToActionNode instances are created/restored
# Look for deserialization or fromJSON patterns
rg -n 'fromJSON|deserialize|restore' packages/koenig-lexical/src/nodes/CallToActionNode.tsx
# Check what BaseCallToActionNode does in constructor
rg -n -A20 'class.*BaseCallToActionNode' --glob '*.ts' --glob '*.tsx' | head -40
# Check if there's any test data or mocking that might show invalid dataset usage
fd '*.test.*' packages/koenig-lexical/src | head -5
fd '*.spec.*' packages/koenig-lexical/src | head -5
# See if there are other places that dispatch INSERT_CALL_TO_ACTION_COMMAND
rg -n 'INSERT_CALL_TO_ACTION_COMMAND' --glob '*.ts' --glob '*.tsx' | grep -v 'import\|export'Repository: TryGhost/Koenig
Length of output: 41
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check base class implementation
rg -n 'class CallToActionNode' packages/koenig-lexical/src/nodes/CallToActionNode.tsx -A 5
# Check for fromJSON method anywhere in the codebase
rg -n 'fromJSON' packages/koenig-lexical/src/ --glob '*.ts' --glob '*.tsx'
# Look at how getDataset() is used - indicates serialization/deserialization
rg -n 'getDataset\|fromJSON' packages/koenig-lexical/src/nodes/ --glob '*.ts' --glob '*.tsx' -lRepository: TryGhost/Koenig
Length of output: 332
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Try to find test files with different patterns
find packages/koenig-lexical/src -name "*.test.*" -o -name "*.spec.*" | head -5
# Look for any externals that provide dataset to these nodes
rg -n 'callToActionHtmlEditor' packages/koenig-lexical/src --glob '*.ts' --glob '*.tsx'
# See what happens if populateNestedEditor is called with a non-LexicalEditor
cat -n packages/koenig-lexical/src/utils/nested-editors.ts | sed -n '44,60p'
# Examine CallToActionNode.getDataset to understand the inverse mapping
cat -n packages/koenig-lexical/src/nodes/CallToActionNode.tsx | sed -n '72,85p'Repository: TryGhost/Koenig
Length of output: 3236
Add runtime validation for LexicalEditor instances before passing to setupNestedEditor.
Type casting dataset.callToActionHtmlEditor as LexicalEditor | undefined only satisfies TypeScript and provides no runtime protection. If dataset contains a non-editor object that is truthy, setupNestedEditor will assign it directly (line 33 does only a truthy check), and later calls to getEditorState() (line 89) will fail at runtime.
This pattern affects multiple node files. Consider adding a type guard:
Suggested fix
+const isLexicalEditor = (value: unknown): value is LexicalEditor => {
+ return typeof value === 'object' &&
+ value !== null &&
+ 'getEditorState' in value &&
+ typeof (value as {getEditorState?: unknown}).getEditorState === 'function';
+};
+
setupNestedEditor(this, '__callToActionHtmlEditor', {
- editor: dataset.callToActionHtmlEditor as LexicalEditor | undefined,
+ editor: isLexicalEditor(dataset.callToActionHtmlEditor) ? dataset.callToActionHtmlEditor : undefined,
nodes: BASIC_NODES
});
setupNestedEditor(this, '__sponsorLabelHtmlEditor', {
- editor: dataset.sponsorLabelHtmlEditor as LexicalEditor | undefined,
+ editor: isLexicalEditor(dataset.sponsorLabelHtmlEditor) ? dataset.sponsorLabelHtmlEditor : undefined,
nodes: BASIC_NODES
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| setupNestedEditor(this, '__callToActionHtmlEditor', {editor: dataset.callToActionHtmlEditor as LexicalEditor | undefined, nodes: BASIC_NODES}); | |
| setupNestedEditor(this, '__sponsorLabelHtmlEditor', {editor: dataset.sponsorLabelHtmlEditor as LexicalEditor | undefined, nodes: BASIC_NODES}); | |
| const isLexicalEditor = (value: unknown): value is LexicalEditor => { | |
| return typeof value === 'object' && | |
| value !== null && | |
| 'getEditorState' in value && | |
| typeof (value as {getEditorState?: unknown}).getEditorState === 'function'; | |
| }; | |
| setupNestedEditor(this, '__callToActionHtmlEditor', {editor: isLexicalEditor(dataset.callToActionHtmlEditor) ? dataset.callToActionHtmlEditor : undefined, nodes: BASIC_NODES}); | |
| setupNestedEditor(this, '__sponsorLabelHtmlEditor', {editor: isLexicalEditor(dataset.sponsorLabelHtmlEditor) ? dataset.sponsorLabelHtmlEditor : undefined, nodes: BASIC_NODES}); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/koenig-lexical/src/nodes/CallToActionNode.tsx` around lines 60 - 61,
The dataset values are being blindly cast to LexicalEditor and passed into
setupNestedEditor, causing runtime failures if the object isn't a real editor;
add a runtime type guard before calling setupNestedEditor for both
dataset.callToActionHtmlEditor and dataset.sponsorLabelHtmlEditor by checking
the presence/type of an editor API (e.g., verify the object is truthy and has a
getEditorState function) and only pass it as the editor when that guard passes,
otherwise pass undefined; update the calls that reference
__callToActionHtmlEditor and __sponsorLabelHtmlEditor so setupNestedEditor
receives a validated LexicalEditor or undefined (this prevents later
getEditorState() calls from throwing).
| const handleImageChange = async (files: File[] | FileList | null) => { | ||
| // reset original src so it can be replaced with preview and upload progress | ||
| editor.update(() => { | ||
| const node = $getNodeByKey(nodeKey); | ||
| const node = $getSignupNodeByKey(nodeKey); | ||
| if (!node) {return;} | ||
| node.backgroundImageSrc = ''; | ||
| }); | ||
|
|
||
| const {imageSrc} = await backgroundImageUploadHandler(files, imageUploader.upload); | ||
| const result = await backgroundImageUploadHandler(Array.from(files!), imageUploader.upload); | ||
| if (!result) {return;} | ||
| const {imageSrc} = result; | ||
|
|
||
| editor.update(() => { | ||
| const node = $getNodeByKey(nodeKey); | ||
| node.backgroundImageSrc = imageSrc; | ||
| const node = $getSignupNodeByKey(nodeKey); | ||
| if (!node) {return;} | ||
| node.backgroundImageSrc = imageSrc ?? ""; | ||
| }); | ||
|
|
||
| setLastBackgroundImage(imageSrc); | ||
| setLastBackgroundImage(imageSrc as string); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cd packages/koenig-lexical && wc -l src/nodes/SignupNodeComponent.tsxRepository: TryGhost/Koenig
Length of output: 96
🏁 Script executed:
cd packages/koenig-lexical && sed -n '120,160p' src/nodes/SignupNodeComponent.tsxRepository: TryGhost/Koenig
Length of output: 1435
🏁 Script executed:
cd packages/koenig-lexical && rg -B 3 -A 10 "backgroundImageUploadHandler" src/Repository: TryGhost/Koenig
Length of output: 7961
🏁 Script executed:
cd packages/koenig-lexical && rg "export.*backgroundImageUploadHandler|const backgroundImageUploadHandler|function backgroundImageUploadHandler" src/Repository: TryGhost/Koenig
Length of output: 221
🏁 Script executed:
cd packages/koenig-lexical && sed -n '1,30p' src/utils/imageUploadHandler.tsRepository: TryGhost/Koenig
Length of output: 1244
🏁 Script executed:
cd packages/koenig-lexical && rg -B 2 -A 8 "useFileDragAndDrop" src/Repository: TryGhost/Koenig
Length of output: 18643
🏁 Script executed:
cd packages/koenig-lexical && sed -n '1,50p' src/hooks/useFileDragAndDrop.tsRepository: TryGhost/Koenig
Length of output: 1602
Move file validation before mutating state.
handleImageChange accepts null (passed from onFileChange via e.target.files), but clears backgroundImageSrc before validating the input. If files is null, Array.from(null) throws. If the upload fails, the image remains cleared. Validate and return early on empty/null input before any state mutations.
Suggested fix
const handleImageChange = async (files: File[] | FileList | null) => {
+ const nextFiles = files ? Array.from(files) : [];
+ if (nextFiles.length === 0) {
+ return;
+ }
+
// reset original src so it can be replaced with preview and upload progress
editor.update(() => {
const node = $getSignupNodeByKey(nodeKey);
if (!node) {return;}
node.backgroundImageSrc = '';
});
- const result = await backgroundImageUploadHandler(Array.from(files!), imageUploader.upload);
+ const result = await backgroundImageUploadHandler(nextFiles, imageUploader.upload);
if (!result) {return;}
const {imageSrc} = result;
editor.update(() => {
const node = $getSignupNodeByKey(nodeKey);
if (!node) {return;}
node.backgroundImageSrc = imageSrc ?? "";
});
- setLastBackgroundImage(imageSrc as string);
+ if (imageSrc) {
+ setLastBackgroundImage(imageSrc);
+ }
setImageRemoved(false);
};🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/koenig-lexical/src/nodes/SignupNodeComponent.tsx` around lines 132 -
150, In handleImageChange, validate the incoming files and return early if files
is null/empty before calling editor.update or Array.from; move the null/empty
check ahead of the initial editor.update so you don't clear
node.backgroundImageSrc when there's nothing to upload, and also avoid calling
Array.from(files) on null. After validating, call
backgroundImageUploadHandler(Array.from(files), imageUploader.upload), then on
successful result run editor.update to set node.backgroundImageSrc and call
setLastBackgroundImage; keep references to $getSignupNodeByKey,
backgroundImageUploadHandler, editor.update, imageUploader.upload, and
setLastBackgroundImage when making the change.
| if (event.key === ':') { | ||
| if (cursorInInlineCodeBlock() === true) { | ||
| return false; | ||
| } | ||
| const emojis = await SearchIndex.search(queryString); | ||
| if (emojis.length === 0) { | ||
| return; | ||
| } | ||
| const emojiMatch = emojis?.[0].id === queryString; // only look for exact match | ||
| if (emojiMatch) { | ||
| handleCompletionInsertion(emojis[0]); | ||
| event.preventDefault(); | ||
| return true; | ||
| } | ||
| SearchIndex.search(queryString).then((emojis: Emoji[]) => { | ||
| if (emojis.length === 0) { | ||
| return; | ||
| } | ||
| const emojiMatch = emojis[0].id === queryString; | ||
| if (emojiMatch) { | ||
| handleCompletionInsertion(emojis[0]); | ||
| } | ||
| }); | ||
| event.preventDefault(); | ||
| return true; |
There was a problem hiding this comment.
Only swallow : after an exact shortcode match is confirmed.
This now prevents the closing colon for any active emoji query, even when there is no exact emoji id. Typing something like :notanemoji: will drop the final : instead of letting the text through.
💡 Suggested fix
if (event.key === ':') {
if (cursorInInlineCodeBlock() === true) {
return false;
}
- SearchIndex.search(queryString).then((emojis: Emoji[]) => {
- if (emojis.length === 0) {
- return;
- }
- const emojiMatch = emojis[0].id === queryString;
- if (emojiMatch) {
- handleCompletionInsertion(emojis[0]);
- }
- });
- event.preventDefault();
- return true;
+ const emojiMatch = searchResults?.find(emoji => emoji.id === queryString);
+ if (!emojiMatch) {
+ return false;
+ }
+ event.preventDefault();
+ handleCompletionInsertion(emojiMatch);
+ return true;
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/koenig-lexical/src/plugins/EmojiPickerPlugin.tsx` around lines 74 -
88, The key handler currently prevents default for ':' unconditionally; change
it so SearchIndex.search(queryString) is awaited (or use its promise) and only
when an exact shortcode match is found (compare emojis[0].id === queryString)
call handleCompletionInsertion(emojis[0]) and then call event.preventDefault()
and return true; otherwise do not preventDefault and return false so the
trailing ':' is inserted normally. Keep the early cursorInInlineCodeBlock()
check as-is and update the branches around SearchIndex.search,
handleCompletionInsertion, and event.preventDefault to only run on confirmed
exact match.
c91da8a to
0f3167a
Compare
There was a problem hiding this comment.
Actionable comments posted: 19
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (9)
packages/koenig-lexical/src/plugins/DragDropPastePlugin.tsx (1)
55-67:⚠️ Potential issue | 🟠 MajorGuard against missing
uploadTypekey inuploadFileTypesto prevent runtime error.On line 61, if
nodeWithUpload.uploadTypeis a truthy string but doesn't exist as a key inuploadFileTypes, accessing.mimeTypesonundefinedwill throw. Additionally, the function is markedasyncbut contains noawaitstatements.Proposed fix
-async function getListOfAcceptableMimeTypes(editor: LexicalEditor, uploadFileTypes: Record<string, {mimeTypes: string[]}>) { +function getListOfAcceptableMimeTypes(editor: LexicalEditor, uploadFileTypes: Record<string, {mimeTypes: string[]}>) { const nodes = getEditorCardNodes(editor); const acceptableMimeTypes: Record<string, string[]> = {}; for (const [nodeType, node] of nodes) { const nodeWithUpload = node as unknown as {uploadType?: string}; - if (nodeType && nodeWithUpload.uploadType) { - acceptableMimeTypes[nodeType] = uploadFileTypes[nodeWithUpload.uploadType].mimeTypes; + if (nodeType && nodeWithUpload.uploadType && uploadFileTypes[nodeWithUpload.uploadType]) { + acceptableMimeTypes[nodeType] = uploadFileTypes[nodeWithUpload.uploadType].mimeTypes; } } return { acceptableMimeTypes }; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/koenig-lexical/src/plugins/DragDropPastePlugin.tsx` around lines 55 - 67, The getListOfAcceptableMimeTypes function can throw if nodeWithUpload.uploadType is present but missing from uploadFileTypes and is unnecessarily async; update it to first read const uploadKey = nodeWithUpload.uploadType, then check both uploadKey and uploadFileTypes[uploadKey] exist before accessing .mimeTypes, assigning a safe fallback (e.g., empty array) to acceptableMimeTypes[nodeType] when missing, and remove the async keyword from getListOfAcceptableMimeTypes if there are no awaits; reference getEditorCardNodes, nodeWithUpload.uploadType, uploadFileTypes and acceptableMimeTypes when making the change.packages/koenig-lexical/src/hooks/useGalleryReorder.ts (1)
107-120:⚠️ Potential issue | 🟡 MinorChained non-null assertions could cause runtime errors.
Line 108 chains two non-null assertions. If the draggable element doesn't contain an
imgor theimglacks asrcattribute, this will throw at runtime.🛡️ Proposed defensive approach
const getDraggableInfo = (draggableElement: HTMLElement) => { - const src = draggableElement.querySelector('img')!.getAttribute('src')!; - const image = images.find(i => i.src === src) || images.find(i => i.previewSrc === src); - const dataset = image && pick(image, ['fileName', 'src', 'row', 'width', 'height', 'caption']); + const imgElement = draggableElement.querySelector('img'); + const src = imgElement?.getAttribute('src'); + if (!src) { + return {}; + } + const image = images.find(i => i.src === src) || images.find(i => i.previewSrc === src); + const dataset = image && pick(image, ['fileName', 'src', 'row', 'width', 'height', 'caption']); if (image) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/koenig-lexical/src/hooks/useGalleryReorder.ts` around lines 107 - 120, In getDraggableInfo, avoid the chained non-null assertions on draggableElement.querySelector('img')!.getAttribute('src')! because querySelector or getAttribute can be null; instead safely query the img with optional chaining and bail out returning {} if no img or no src, then lookup image via images.find(...) and only call pick(image, [...]) when image is truthy—remove the "!" assertions and add explicit null/undefined checks around src, image and dataset to prevent runtime errors.packages/koenig-lexical/src/nodes/AudioNodeComponent.tsx (1)
8-8:⚠️ Potential issue | 🟠 MajorUpdate import to use correct file extension or extensionless path.
Line 8 imports from
SnippetActionToolbar.jsx, but the actual file isSnippetActionToolbar.tsx. Use either the correct.tsxextension or an extensionless import path to align with TypeScript migration standards.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/koenig-lexical/src/nodes/AudioNodeComponent.tsx` at line 8, Update the import in AudioNodeComponent.tsx to reference the actual module file by changing the import source from 'SnippetActionToolbar.jsx' to the correct 'SnippetActionToolbar.tsx' or use an extensionless path 'SnippetActionToolbar'; ensure the import statement that brings in SnippetActionToolbar from '../components/ui/SnippetActionToolbar.jsx' is updated so it matches the real filename and TypeScript module resolution.packages/koenig-lexical/src/components/ui/KeyboardSelectionWithGroups.tsx (1)
58-62:⚠️ Potential issue | 🟡 MinorPotential undefined access when items array is empty.
On Line 61,
onSelect(items[selectedIndex])could passundefinedifitemsis empty (sinceselectedIndexdefaults to 0 viaMath.max(0, ...)). TheonSelectcallback receivesunknown, so this won't cause a type error but could cause runtime issues in consumers.Consider adding a guard:
Proposed fix
if (event.key === 'Enter') { event.preventDefault(); event.stopPropagation(); - onSelect(items[selectedIndex]); + if (items[selectedIndex]) { + onSelect(items[selectedIndex]); + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/koenig-lexical/src/components/ui/KeyboardSelectionWithGroups.tsx` around lines 58 - 62, The Enter key handler in KeyboardSelectionWithGroups calls onSelect(items[selectedIndex]) which can pass undefined when items is empty; update the handler in that component to guard before invoking onSelect by checking items.length > 0 and that selectedIndex is within [0, items.length-1], and only call onSelect(items[selectedIndex]) when that condition holds (otherwise no-op or early return); reference the Enter-key branch in the event handler, the items array and selectedIndex variables, and the onSelect callback.packages/koenig-lexical/src/hooks/useSearchLinks.ts (1)
70-84:⚠️ Potential issue | 🟡 MinorType mismatch in converted items.
The mapped items (Lines 72-80) include all
ListItemfields excepthighlight, which is required by theListIteminterface but not present inSearchResultItem. This could cause issues if downstream code expectshighlightto be defined.Proposed fix: Add default highlight value
return { label: item.title, value: item.url, Icon: item.Icon, metaText: item.metaText, MetaIcon: item.MetaIcon, metaIconTitle: item.metaIconTitle, + highlight: false, type: type || 'internal' };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/koenig-lexical/src/hooks/useSearchLinks.ts` around lines 70 - 84, The mapped items created inside results.map -> result.items.map are missing the required ListItem.highlight field (SearchResultItem lacks it), so update the mapping in useSearchLinks (the returned items construction) to include a default highlight value (e.g. empty string or appropriate default per ListItem) for each item so the produced objects satisfy the ListItem interface; ensure you add highlight alongside label/value/Icon/metaText/MetaIcon/metaIconTitle/type.packages/koenig-lexical/src/components/ui/LinkActionToolbarWithSearch.tsx (1)
1-2:⚠️ Potential issue | 🟡 MinorUpdate imports to reference
.tsxextensions instead of.jsx.Lines 1-2 import from
.jsxfiles, but the actual files exist as.tsx(KoenigComposerContext.tsx and Portal.tsx). Update the imports to use the correct extensions:
'../../context/KoenigComposerContext.jsx'→'../../context/KoenigComposerContext.tsx''./Portal.jsx'→'./Portal.tsx'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/koenig-lexical/src/components/ui/LinkActionToolbarWithSearch.tsx` around lines 1 - 2, Update the two incorrect import paths in LinkActionToolbarWithSearch.tsx so they reference the actual .tsx modules: replace the import of KoenigComposerContext from '../../context/KoenigComposerContext.jsx' with '../../context/KoenigComposerContext.tsx' and replace the import of Portal from './Portal.jsx' with './Portal.tsx' so the imports match the real component files (look for the import statements referencing KoenigComposerContext and Portal).packages/koenig-lexical/src/components/ui/UrlInput.tsx (1)
83-91:⚠️ Potential issue | 🟡 MinorRemove redundant onKeyDown handler to prevent duplicate submission.
handleUrlSubmitis called twice on Enter: once via theUrlInputPlugin's LexicalKEY_ENTER_COMMAND(line 83) and again via the input's nativeonKeyDown(line 91). Since the Lexical command handler returnsfalse, the native event propagates unblocked.Remove the
onKeyDown={handleUrlSubmit}from the input element—theUrlInputPluginalready handles Enter submission.Proposed fix
<input autoFocus={true} className="w-full rounded-md border border-grey-300 p-2 font-sans text-sm font-normal leading-snug text-grey-900 focus-visible:outline-none dark:border-grey-800 dark:bg-grey-950 dark:text-grey-100 dark:placeholder:text-grey-800" data-testid={dataTestId} placeholder={placeholder} value={value} onChange={handleUrlChange} - onKeyDown={handleUrlSubmit} />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/koenig-lexical/src/components/ui/UrlInput.tsx` around lines 83 - 91, The input element is triggering duplicate submissions because Enter is handled both by UrlInputPlugin's Lexical KEY_ENTER_COMMAND and by the input's native onKeyDown; remove the onKeyDown={handleUrlSubmit} prop from the input JSX so only UrlInputPlugin invokes handleUrlSubmit (locate the input element in UrlInput.tsx and remove its onKeyDown handler).packages/koenig-lexical/src/nodes/ProductNodeComponent.tsx (1)
70-89:⚠️ Potential issue | 🟠 MajorUpload only the selected file, and always clear the preview URL in
finally.This handler previews
files[0]but uploadsArray.from(files), so a multi-file drop will upload extra images that are never attached to the node. Also, any rejection fromgetImageDimensions()orimgUploader.upload()exits before the preview URL is cleared/revoked, leaving an unsaved blob preview on screen.Suggested change
const handleImgUpload = async (files: File[] | FileList) => { - const imgPreviewUrl = URL.createObjectURL(files[0]); + const file = files[0]; + if (!file) { + return; + } + + const imgPreviewUrl = URL.createObjectURL(file); setImgPreview(imgPreviewUrl); - const {width, height} = await getImageDimensions(imgPreviewUrl); - const imgUploadResult = await imgUploader.upload(Array.from(files)); - const imageUrl = imgUploadResult?.[0]?.url; - - if (imageUrl) { - editor.update(() => { - const node = $getProductNodeByKey(nodeKey); - if (!node) {return;} - node.productImageSrc = imageUrl; - node.productImageHeight = height; - node.productImageWidth = width; - }); + try { + const {width, height} = await getImageDimensions(imgPreviewUrl); + const imgUploadResult = await imgUploader.upload([file]); + const imageUrl = imgUploadResult?.[0]?.url; + + if (imageUrl) { + editor.update(() => { + const node = $getProductNodeByKey(nodeKey); + if (!node) {return;} + node.productImageSrc = imageUrl; + node.productImageHeight = height; + node.productImageWidth = width; + }); + } + } finally { + setImgPreview(''); + URL.revokeObjectURL(imgPreviewUrl); } - - setImgPreview(''); - URL.revokeObjectURL(imgPreviewUrl); };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/koenig-lexical/src/nodes/ProductNodeComponent.tsx` around lines 70 - 89, In handleImgUpload: only upload the selected first file (use files[0] rather than Array.from(files)) and wrap the async work (getImageDimensions and imgUploader.upload and subsequent editor.update using nodeKey) in a try/finally so that setImgPreview('') and URL.revokeObjectURL(imgPreviewUrl) always run even if getImageDimensions or imgUploader.upload throws; ensure you still set node.productImageSrc/Width/Height inside the successful path before finally clearing the preview.packages/koenig-lexical/src/components/KoenigComposer.tsx (1)
60-92:⚠️ Potential issue | 🟠 MajorReuse the normalized
initialEditorStatein collaboration mode.Lines 64-84 normalize empty roots to a serialized editor-state string, but Lines 151-152 still pass the raw prop into
CollaborationPlugin. That meansinitialEditorState={{...}}behaves differently whenenableMultiplayeris true, and the empty-root fix is skipped on that path.Also applies to: 149-152
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/koenig-lexical/src/components/KoenigComposer.tsx` around lines 60 - 92, The normalization that ensures a non-empty root is applied to the local editorState in initialConfig but is not reused when enableMultiplayer is true; update KoenigComposer so the normalized/serialized initial state (the same value computed as editorState in the initialConfig memo) is passed into CollaborationPlugin instead of the raw initialEditorState prop. Locate the initialConfig useMemo and the CollaborationPlugin instantiation, and change the prop passed to CollaborationPlugin (initialEditorState or equivalent) to use the normalized value from initialConfig (or expose that normalized variable) so the empty-root fix is applied in collaboration mode as well.
♻️ Duplicate comments (1)
packages/koenig-lexical/src/nodes/SignupNodeComponent.tsx (1)
132-150:⚠️ Potential issue | 🔴 CriticalStill validate
filesbefore clearing the image.
filescan still benullhere, soArray.from(files!)will throw afterbackgroundImageSrchas already been cleared. A failed upload also leaves the card blank. Please keep the existing image until you have a non-empty file list and a successful upload result.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/koenig-lexical/src/nodes/SignupNodeComponent.tsx` around lines 132 - 150, handleImageChange currently clears node.backgroundImageSrc before validating files and uploading, which can leave the card blank if files is null/empty or upload fails; change handleImageChange to first validate that files is non-null and has length (guard Array.from(files) without using the non-null assertion), return early if not, then call backgroundImageUploadHandler(imageUploader.upload) and only on a successful result update the editor node via $getSignupNodeByKey to set node.backgroundImageSrc and call setLastBackgroundImage; ensure no editor update clears the image unless you have a confirmed new imageSrc from backgroundImageUploadHandler.
🟡 Minor comments (23)
packages/koenig-lexical/src/components/ui/TabView.tsx-14-15 (1)
14-15:⚠️ Potential issue | 🟡 MinorPotential runtime error if
tabsarray is empty.If
tabsis passed as an empty array anddefaultTabis not provided, accessingtabs[0].idwill throw aTypeErrorsincetabs[0]isundefined. Consider adding a guard or makingtabsrequire at least one element.🛡️ Proposed defensive fix
const TabView = ({tabs, defaultTab, tabContent}: TabViewProps) => { - const [activeTab, setActiveTab] = useState(defaultTab || tabs[0].id); + const [activeTab, setActiveTab] = useState(defaultTab || tabs[0]?.id || '');Alternatively, if
tabsshould never be empty, consider typing it more strictly or adding a runtime assertion.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/koenig-lexical/src/components/ui/TabView.tsx` around lines 14 - 15, The TabView component initializes activeTab with tabs[0].id which will throw if tabs is an empty array; update the TabView initialization to defensively choose defaultTab || (tabs && tabs.length ? tabs[0].id : someFallback) or assert tabs has at least one element before calling useState so activeTab is always defined; locate the TabView function and the useState call (activeTab, setActiveTab) and either add a guard that throws/returns early when tabs.length === 0 or provide a safe fallback id for the initial state.packages/koenig-lexical/src/components/ui/cards/MarkdownCard/MarkdownEditor.tsx-257-257 (1)
257-257:⚠️ Potential issue | 🟡 MinorType cast to
unknownhides a signature mismatch.The callback
onUnsplashInsertexpects{src: string; alt?: string; caption: string}, butUnsplashSearchModalprovidesInsertImagePayloadwith signature{src: string; caption: string; height: number; width: number; alt: string}(requiredalt, plus additionalheightandwidthproperties). Casting to(image: unknown) => voidsuppresses this type error.Align the signatures by either:
- Updating
onUnsplashInsertto accept the fullInsertImagePayloadshape, or- Having
UnsplashModaldefine a strict prop type instead ofunknownto enforce compatibility at the component boundary.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/koenig-lexical/src/components/ui/cards/MarkdownCard/MarkdownEditor.tsx` at line 257, The onUnsplashInsert callback is being cast to (image: unknown) => void which hides a signature mismatch with UnsplashSearchModal's InsertImagePayload (src, caption, height, width, alt); update the types so they match: either change the onUnsplashInsert function signature to accept InsertImagePayload (including required alt and optional/required height/width as appropriate) or tighten UnsplashSearchModal's prop type to only supply the expected {src: string; alt?: string; caption: string}; update the prop/type declarations where UnsplashSearchModal is defined and where onUnsplashInsert is declared/used to remove the unsafe cast and ensure the types align (refer to onUnsplashInsert, UnsplashSearchModal, and InsertImagePayload).packages/koenig-lexical/src/components/ui/cards/CodeBlockCard.tsx-205-213 (1)
205-213:⚠️ Potential issue | 🟡 MinorProvide fallback value for controlled input.
value={language}may receiveundefined, which can cause React to treat the input as uncontrolled initially and then controlled when a value is set, triggering a warning.Proposed fix
<input aria-label="Code card language" className={`z-999 absolute right-1.5 top-1.5 w-1/5 rounded-md border border-grey-300 px-2 py-1 font-sans text-[1.3rem] leading-4 text-grey-900 transition-opacity focus-visible:outline-none dark:border-grey-900 dark:text-grey-400 ${showLanguage ? 'opacity-100' : 'opacity-0'}`} data-testid="code-card-language" placeholder="Language..." type="text" - value={language} + value={language || ''} onChange={onLanguageChange} />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/koenig-lexical/src/components/ui/cards/CodeBlockCard.tsx` around lines 205 - 213, The input in CodeBlockCard uses value={language} which can be undefined causing a controlled/uncontrolled React warning; change the input to use a stable string fallback (e.g. use language ?? '' or String(language)) so the value is always a string, and ensure onLanguageChange continues to update the prop/state; update the <input> with this fallback and keep references to showLanguage, data-testid="code-card-language", and onLanguageChange intact.packages/koenig-lexical/src/components/ui/cards/CodeBlockCard.tsx-15-21 (1)
15-21:⚠️ Potential issue | 🟡 MinorRemove unused
darkModeprop from interface.The
darkModeprop is defined in the interface but never destructured or used inCodeEditor(line 23). The component retrievesdarkModefromKoenigComposerContextinstead. This causes confusion sinceCodeBlockCardpassesdarkModetoCodeEditor(line 259) but it's silently ignored.Either remove the unused prop or use it instead of the context value.
Proposed fix to remove unused prop
interface CodeEditorProps { code?: string; language?: string; updateCode?: (value: string) => void; updateLanguage?: (value: string) => void; - darkMode?: boolean; }And in
CodeBlockCard:<CodeEditor code={code} - darkMode={darkMode} language={language} updateCode={updateCode} updateLanguage={updateLanguage} />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/koenig-lexical/src/components/ui/cards/CodeBlockCard.tsx` around lines 15 - 21, The CodeEditorProps interface includes an unused darkMode property which is never read by the CodeEditor component (it obtains darkMode from KoenigComposerContext) while CodeBlockCard still passes darkMode into CodeEditor; remove the unused prop to avoid confusion: delete darkMode from the CodeEditorProps interface and stop passing darkMode from CodeBlockCard to CodeEditor (or alternatively, if you prefer prop-driven behavior, update the CodeEditor function to read and use the darkMode prop instead of the context). Update references to the prop in CodeEditor, CodeEditorProps, and the CodeBlockCard component so the signature and usage are consistent (prefer removing the prop for the simpler change).packages/koenig-lexical/src/components/ui/cards/VideoCard.tsx-115-116 (1)
115-116:⚠️ Potential issue | 🟡 MinorNon-null assertions on optional props could cause silent runtime failures.
onCardWidthChange!andonCustomThumbnailChange!use non-null assertions on props typed as optional. If a caller omits these props, TypeScript won't warn, but clicking these controls will invokeundefined().Consider either making these props required (if they're always needed) or adding runtime guards.
Option 1: Make props required when editing
interface PopulatedVideoCardProps { // ... other props - onCardWidthChange?: (name: string) => void; + onCardWidthChange: (name: string) => void; // ... - onCustomThumbnailChange?: (e: React.ChangeEvent<HTMLInputElement>) => void; + onCustomThumbnailChange: (e: React.ChangeEvent<HTMLInputElement>) => void; }Option 2: Add runtime guards
- onClick={onCardWidthChange!} + onClick={onCardWidthChange ?? (() => {})} ... - onFileChange={onCustomThumbnailChange!} + onFileChange={onCustomThumbnailChange ?? (() => {})}Also applies to: 139-139
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/koenig-lexical/src/components/ui/cards/VideoCard.tsx` around lines 115 - 116, The component uses non-null assertions on optional props (onCardWidthChange and onCustomThumbnailChange) which can cause runtime errors if the props are omitted; update VideoCard.tsx to remove the "!" usages and add runtime guards before invoking them (e.g., if (onCardWidthChange) onCardWidthChange(...)) or alternatively make the props required in the component props interface and update callers accordingly; ensure you update every call site in this file where onCardWidthChange and onCustomThumbnailChange are invoked (including where selectedName={cardWidth || ''} is passed) so no invocation assumes the prop is non-null.packages/koenig-lexical/src/hooks/useMovable.ts-74-76 (1)
74-76:⚠️ Potential issue | 🟡 MinorNon-null assertion risk in exported
setPositioncallback chain.
setTranslate(and by extensionsetPosition) usesref.current!which will throw if the ref isn't attached. SincesetPositionis returned from the hook and can be called externally, consider adding a guard or returning early whenref.currentis null.🛡️ Proposed defensive check
const setTranslate = useCallback((xPos: number, yPos: number) => { + if (!ref.current) { + return; + } - ref.current!.style.transform = `translate(${xPos}px, ${yPos}px)`; + ref.current.style.transform = `translate(${xPos}px, ${yPos}px)`; }, [ref]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/koenig-lexical/src/hooks/useMovable.ts` around lines 74 - 76, Replace the unsafe non-null assertion in the useCallback for setTranslate (and thus exported setPosition) with a defensive null check: in the setTranslate callback (and any functions that call it like setPosition), test if ref.current is present and return early if it's null to avoid throwing; remove the use of ref.current! and bail out gracefully (no-op) when the ref isn't attached so external callers of setPosition won't crash.packages/koenig-lexical/src/hooks/useMovable.ts-268-273 (1)
268-273:⚠️ Potential issue | 🟡 MinorInconsistent null handling within the same effect.
Line 269 uses non-null assertion (
ref.current!) but line 271 uses optional chaining (ref.current?.classList). Ifref.currentcan be null, the assertion on line 269 would throw. Consider adding an early return guard instead.🛡️ Proposed fix for consistent null handling
useEffect(() => { - const elem = ref.current!; + if (!ref.current) { + return; + } + const elem = ref.current; elem.setAttribute('draggable', 'true'); - ref.current?.classList.add('kg-card-movable'); + elem.classList.add('kg-card-movable'); let _resizeObserver: ResizeObserver | undefined;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/koenig-lexical/src/hooks/useMovable.ts` around lines 268 - 273, The effect in useMovable uses inconsistent null handling (ref.current! vs ref.current?.classList); change to a single guarded path: capture const elem = ref.current, if (!elem) return early from the effect, then use elem.setAttribute and elem.classList.add consistently; ensure subsequent references (e.g., when creating _resizeObserver and calling addStartEventListeners) use the guarded elem or ref safely so no non-null assertions are needed.packages/koenig-lexical/src/hooks/useSettingsPanelReposition.ts-242-245 (1)
242-245:⚠️ Potential issue | 🟡 MinorDebounced callback may access stale ref after unmount.
The debounced
panelRepositionDebouncedcallback usesref.current!but could fire after the component unmounts (debounce trailing edge). Consider canceling the debounce on cleanup.🛡️ Proposed fix to cancel debounce on cleanup
const resizeObserver = new ResizeObserver((entries) => { for (const entry of entries) { if (entry.contentBoxSize?.[0]) { const width = entry.contentBoxSize[0].inlineSize; if (typeof width === 'number' && width !== prevWidth) { panelRepositionDebounced(width); } } } }); resizeObserver.observe(container); return () => { + panelRepositionDebounced.cancel(); resizeObserver.disconnect(); };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/koenig-lexical/src/hooks/useSettingsPanelReposition.ts` around lines 242 - 245, The debounced callback panelRepositionDebounced can run after unmount and access ref.current; update the hook to cancel the debounce on cleanup by calling panelRepositionDebounced.cancel() (or the equivalent cancel method from your debounce impl) in the effect or hook teardown where panelRepositionDebounced is created, ensuring no trailing invocation will call onResize(ref.current) after unmount; reference the debounce call that creates panelRepositionDebounced and the onResize/ref usage so you place the cancel in the same scope where panelRepositionDebounced is defined.packages/koenig-lexical/src/hooks/useMovable.ts-97-103 (1)
97-103:⚠️ Potential issue | 🟡 Minor
getPositionmay returnundefinedvalues despite typed return.
currentX.currentandcurrentY.currentstart asundefinedand are only set aftersetPositionis called. Using non-null assertions here means the function could returnundefinedcast asnumberif called before positioning is initialized.🛡️ Proposed fix with explicit typing
-const getPosition = useCallback(() => { +const getPosition = useCallback((): Position | undefined => { + if (currentX.current === undefined || currentY.current === undefined) { + return undefined; + } return { - x: currentX.current!, - y: currentY.current!, + x: currentX.current, + y: currentY.current, lastSpacing: lastSpacing.current }; }, []);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/koenig-lexical/src/hooks/useMovable.ts` around lines 97 - 103, getPosition currently uses non-null assertions on currentX.current and currentY.current which start as undefined, so it can return undefined cast to number; remove the '!' and either return a typed union or safe defaults: update getPosition's return shape (e.g., { x?: number; y?: number; lastSpacing: number } or { x: number; y: number; lastSpacing: number } with defaults) and replace currentX.current! and currentY.current! with currentX.current ?? undefined (or ?? 0 if you choose defaults), and ensure callers of getPosition (and any types) are updated to accept optional x/y or rely on the chosen defaults; reference: getPosition, currentX, currentY, lastSpacing, setPosition.packages/koenig-lexical/src/components/ui/Dropdown.tsx-43-49 (1)
43-49:⚠️ Potential issue | 🟡 MinorUse
event.currentTargetinstead ofevent.targetfor reliable button focus.When the user clicks the SVG icon inside the button,
event.targetrefers to the SVG element, not the button. This causesfocus()to be called on a non-focusable element. Useevent.currentTargetto reliably focus the button regardless of click location within its subtree.Suggested fix
- const handleOpen = (event: React.MouseEvent) => { - setOpen(!open); + const handleOpen = (event: React.MouseEvent<HTMLButtonElement>) => { + setOpen(prev => !prev); // For Safari, we need to manually focus the button (doesn't happen by default) if (!open) { - (event.target as HTMLElement).focus(); + event.currentTarget.focus(); } };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/koenig-lexical/src/components/ui/Dropdown.tsx` around lines 43 - 49, The click handler handleOpen currently uses (event.target as HTMLElement).focus(), which can focus an inner SVG instead of the button; change this to use event.currentTarget (e.g., (event.currentTarget as HTMLElement).focus()) so the button itself is focused when opening; keep the setOpen(!open) behavior and only call focus when transitioning from closed to open.packages/koenig-lexical/src/nodes/ImageNodeComponent.tsx-210-215 (1)
210-215:⚠️ Potential issue | 🟡 MinorDuplicate null guards in triggerFileDialog and handleImageCardResize.
Same pattern of duplicate guards.
Remove duplicate guards
editor.update(() => { const node = $getImageNodeByKey(nodeKey); - if (!node) {return;} if (!node) {return;} node.triggerFileDialog = false; });editor.update(() => { const node = $getImageNodeByKey(nodeKey); - if (!node) {return;} if (!node) {return;} node.cardWidth = newWidth; setCardWidth(newWidth); });Also applies to: 228-235
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/koenig-lexical/src/nodes/ImageNodeComponent.tsx` around lines 210 - 215, There are duplicate null/undefined checks for the image node in blocks using editor.update and $getImageNodeByKey (e.g., the block that sets node.triggerFileDialog = false and the handleImageCardResize block); remove the redundant repeated guard so each block performs a single check like "const node = $getImageNodeByKey(nodeKey); if (!node) return;" before mutating node properties (e.g., node.triggerFileDialog) and similarly in handleImageCardResize to keep one null guard per block while preserving the early return behavior.packages/koenig-lexical/src/nodes/ImageNodeComponent.tsx-180-196 (1)
180-196:⚠️ Potential issue | 🟡 MinorDuplicate null guards in setHref and setAltText.
Both functions have the same duplicate guard pattern.
Remove duplicate guards in both functions
const setHref = (newHref: string) => { editor.update(() => { const node = $getImageNodeByKey(nodeKey); - if (!node) {return;} if (!node) {return;} node.href = newHref; }); }; const setAltText = (newAltText: string) => { editor.update(() => { const node = $getImageNodeByKey(nodeKey); - if (!node) {return;} if (!node) {return;} node.alt = newAltText; }); };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/koenig-lexical/src/nodes/ImageNodeComponent.tsx` around lines 180 - 196, Both setHref and setAltText contain duplicate null guards; inside the editor.update callbacks call $getImageNodeByKey(nodeKey) once and keep a single check "if (!node) { return; }" before setting node.href or node.alt. Update the functions setHref and setAltText to remove the repeated identical guard so each callback only performs one null-check on the node obtained from $getImageNodeByKey(nodeKey) and then assigns newHref to node.href or newAltText to node.alt respectively.packages/koenig-lexical/src/nodes/ImageNodeComponent.tsx-170-178 (1)
170-178:⚠️ Potential issue | 🟡 MinorDuplicate null guards - dead code.
Same issue: two identical
if (!node) {return;}checks in succession.Remove duplicate guard
editor.update(() => { const node = $getImageNodeByKey(nodeKey); - if (!node) {return;} if (!node) {return;} node.src = ''; });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/koenig-lexical/src/nodes/ImageNodeComponent.tsx` around lines 170 - 178, The editor.update block contains two identical null-guards for the retrieved node which is dead code; inside the callback that calls $getImageNodeByKey(nodeKey) remove the duplicated `if (!node) { return; }` so only a single null-check remains before setting `node.src = ''`; update the code in ImageNodeComponent.tsx (inside the editor.update callback around $getImageNodeByKey and nodeKey) so it has one null guard and then assigns node.src.packages/koenig-lexical/src/nodes/ImageNodeComponent.tsx-140-145 (1)
140-145:⚠️ Potential issue | 🟡 MinorDuplicate null guards - dead code.
There are two identical
if (!node) {return;}checks in succession. The second check is unreachable dead code.Remove duplicate guards
editor.update(() => { const node = $getImageNodeByKey(nodeKey); - if (!node) {return;} if (!node) {return;} node.width = width; node.height = height; });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/koenig-lexical/src/nodes/ImageNodeComponent.tsx` around lines 140 - 145, Remove the duplicate null guard: in the block that calls $getImageNodeByKey(nodeKey) and assigns to node, keep a single check for if (!node) { return; } and remove the redundant second identical check so the code proceeds to set node.width and node.height; the symbols to update are $getImageNodeByKey, node, nodeKey, node.width and node.height.packages/koenig-lexical/src/components/ui/cards/ImageCard.tsx-123-123 (1)
123-123:⚠️ Potential issue | 🟡 MinorNon-null assertion on optional prop may cause runtime error.
onFileChangeis typed as optional (onFileChange?: ...) in bothEmptyImageCardPropsandImageHolderProps, but is used with a non-null assertion here. IfEmptyImageCardis ever rendered withoutonFileChange, this will throw at runtime.Suggested fix: Guard or require the prop
Either make
onFileChangerequired inEmptyImageCardPropssince the component cannot function without it:interface EmptyImageCardProps { - onFileChange?: (e: React.ChangeEvent<HTMLInputElement>) => void; + onFileChange: (e: React.ChangeEvent<HTMLInputElement>) => void; setFileInputRef: (ref: React.RefObject<HTMLInputElement | null>) => void; imageFileDragHandler?: DragHandler; errors?: {message: string}[]; }Or guard the usage:
- onFileChange={onFileChange!} + onFileChange={onFileChange || (() => {})}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/koenig-lexical/src/components/ui/cards/ImageCard.tsx` at line 123, EmptyImageCard uses a non-null assertion when passing onFileChange to ImageHolder (onFileChange!), but onFileChange is optional in EmptyImageCardProps and ImageHolderProps and can cause a runtime crash; fix by either making onFileChange required in EmptyImageCardProps and ImageHolderProps (so the prop must be provided) or removing the non-null assertion and guarding the call (e.g., only pass onFileChange when it exists or supply a safe no-op handler) — update the prop types (EmptyImageCardProps, ImageHolderProps) and the usage inside EmptyImageCard/ImageCard to match the chosen approach.packages/koenig-lexical/src/components/ui/FloatingLinkToolbar.tsx-33-41 (1)
33-41:⚠️ Potential issue | 🟡 MinorAvoid blind casting of
event.targetwithout runtime type validation.
event.targetfromMouseEventis typed asEventTargetand is not guaranteed to be anHTMLElement. The current cast on line 40 can store values that don't match theHTMLElement | nullcontract, risking runtime errors whengetClientRects()is called downstream.Suggested fix
const onMouseEnter = (event: MouseEvent) => { + const target = event.target; + if (!(target instanceof HTMLElement)) { + return; + } if (toolbarRef.current?.contains(event.target as Node)) { return; } editor.update(() => { - const node = $getNearestNodeFromDOMNode(event.target as Node); - setTargetElem(event.target as HTMLElement); + const node = $getNearestNodeFromDOMNode(target); + setTargetElem(target);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/koenig-lexical/src/components/ui/FloatingLinkToolbar.tsx` around lines 33 - 41, The onMouseEnter handler blindly casts event.target to HTMLElement; change it to first guard with a runtime check (e.g., if (!(event.target instanceof HTMLElement)) return) before calling toolbarRef.current?.contains or setTargetElem and before passing to $getNearestNodeFromDOMNode; ensure setTargetElem only receives HTMLElement | null and any downstream calls that assume getClientRects() are protected by the same instanceof check so you don't store non-HTMLElement values.packages/koenig-lexical/src/components/ui/FloatingFormatToolbar.tsx-58-68 (1)
58-68:⚠️ Potential issue | 🟡 MinorGuard
event.targetbefore callingcontains().This block assumes
e.targetis always aNodeand dereferences it directly. A null/non-Nodetarget will throw at runtime.Suggested fix
- const toggle = (e: MouseEvent | TouchEvent) => { + const toggle = (e: MouseEvent | TouchEvent) => { + const target = e.target; + if (!(target instanceof Node)) { + return; + } editor.getEditorState().read(() => { const selection = $getSelection(); if ($isRangeSelection(selection)) { const selectedNodeMatchesTarget = selection.getNodes().find((node) => { const element = editor.getElementByKey(node.getKey()); - return element && (element.contains(e.target as Node) || (e.target as Node).contains(element)); + return element && (element.contains(target) || target.contains(element)); }); if (selectedNodeMatchesTarget) { - showToolbarIfHidden(e as MouseEvent); + showToolbarIfHidden(); } } }); };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/koenig-lexical/src/components/ui/FloatingFormatToolbar.tsx` around lines 58 - 68, The toggle handler assumes e.target is always a Node which can cause runtime errors; update the logic in toggle (used with editor.getEditorState().read) to guard the event target before calling contains(): obtain the target from the MouseEvent|TouchEvent, verify it is non-null and is a Node (e.g., instanceof Node) and only then compute selectedNodeMatchesTarget by calling editor.getElementByKey(node.getKey()) and using element.contains(target) || target.contains(element); keep the existing call to showToolbarIfHidden when a match is found.packages/koenig-lexical/src/hooks/useKoenigTextEntity.ts-143-148 (1)
143-148:⚠️ Potential issue | 🟡 MinorTuple casts on
splitTextresults assume specific return length.The casts
as [TextNodeType, TextNodeType]andas [TextNodeType, TextNodeType, TextNodeType]assumesplitTextalways returns exactly 2 or 3 elements. IfsplitTextreturns fewer elements (e.g., at boundary conditions), this will cause runtime errors when accessing array indices.Consider adding defensive checks or using optional chaining on array access.
Proposed defensive approach
if (match.start === 0) { - [nodeToReplace, currentNode] = currentNode!.splitText(match.end) as [TextNodeType, TextNodeType]; + const splitResult = currentNode!.splitText(match.end); + nodeToReplace = splitResult[0] as TextNodeType; + currentNode = (splitResult[1] as TextNodeType) ?? null; } else {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/koenig-lexical/src/hooks/useKoenigTextEntity.ts` around lines 143 - 148, The tuple casts on currentNode!.splitText in useKoenigTextEntity.ts (where nodeToReplace and currentNode are assigned) assume splitText returns exactly 2 or 3 items and can crash at boundaries; change this to defensively read the returned array (e.g., const parts = currentNode!.splitText(...)) and verify parts.length before indexing or destructuring, then assign nodeToReplace and currentNode from parts with safe fallbacks (or return/abort if parts are insufficient), and use optional chaining when accessing parts elements so splitText boundary cases do not cause runtime errors.packages/koenig-lexical/src/hooks/useKoenigTextEntity.ts-47-49 (1)
47-49:⚠️ Potential issue | 🟡 MinorUse public
getMode()method instead of accessing internal__modeproperty.The cast
(node.getLatest() as unknown as {__mode: number}).__modeaccesses an internal property marked@internalin Lexical. The public API providesTextNode.getMode()which returns the mode as a string ("normal" | "token" | "segmented").Update the
getModefunction to use the public method and change the comparison at line 67 fromgetMode(prevSibling) !== 0togetMode(prevSibling) !== "normal".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/koenig-lexical/src/hooks/useKoenigTextEntity.ts` around lines 47 - 49, The getMode helper currently reads the internal __mode field from node.getLatest(); replace that with the public TextNode.getMode() API: cast the incoming LexicalNode to a TextNode (or use node.getLatest() as TextNode) and call getMode() to return the string union ("normal" | "token" | "segmented"); then update the comparison that checks sibling mode (previously comparing getMode(prevSibling) !== 0) to compare against the string "normal" (getMode(prevSibling) !== "normal") so the code uses the public API and string modes instead of internal numeric __mode.packages/koenig-lexical/src/nodes/ProductNode.tsx-47-60 (1)
47-60:⚠️ Potential issue | 🟡 MinorValidate
datasettext fields before populating nested editors.The constructor accepts
Record<string, unknown>, butpopulateNestedEditor()expectsstring | undefined. The current code uses template literals to coerce values, which will convert objects to"[object Object]"instead of preventing malformed payloads.Suggested change
constructor(dataset: Record<string, unknown> = {}, key?: string) { super(dataset, key); + const productTitle = typeof dataset.productTitle === 'string' ? dataset.productTitle : undefined; + const productDescription = typeof dataset.productDescription === 'string' ? dataset.productDescription : undefined; // set up nested editor instances setupNestedEditor(this, '__productTitleEditor', {editor: dataset.productTitleEditor as LexicalEditor | undefined, nodes: MINIMAL_NODES}); setupNestedEditor(this, '__productDescriptionEditor', {editor: dataset.productDescriptionEditor as LexicalEditor | undefined, nodes: BASIC_NODES}); // populate nested editors on initial construction - if (!dataset.productTitleEditor && dataset.productTitle) { - populateNestedEditor(this, '__productTitleEditor', `${dataset.productTitle}`); // we serialize with no wrapper + if (!dataset.productTitleEditor && productTitle) { + populateNestedEditor(this, '__productTitleEditor', productTitle); // we serialize with no wrapper } if (!dataset.productDescriptionEditor) { - populateNestedEditor(this, '__productDescriptionEditor', dataset.productDescription as string | undefined); + populateNestedEditor(this, '__productDescriptionEditor', productDescription); } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/koenig-lexical/src/nodes/ProductNode.tsx` around lines 47 - 60, The constructor is coercing arbitrary dataset fields to strings (e.g., using `${dataset.productTitle}`) which can produce "[object Object]" for non-string payloads; update the constructor logic around setupNestedEditor/populateNestedEditor so you only call populateNestedEditor when the source is actually a string: check typeof dataset.productTitle === 'string' before calling populateNestedEditor(this, '__productTitleEditor', dataset.productTitle), and check typeof dataset.productDescription === 'string' before calling populateNestedEditor(this, '__productDescriptionEditor', dataset.productDescription); remove the template literal coercion and ensure you pass undefined (or omit the call) for non-string values to avoid serializing malformed payloads.packages/koenig-lexical/src/components/ui/cards/HeaderCard/v2/HeaderCard.tsx-109-111 (1)
109-111:⚠️ Potential issue | 🟡 Minor
handleButtonUrlBlurbecame dead code here.The parent still passes a blur handler that restores the default URL, but this component discards it and never forwards an
onBlurtoInputUrlSetting. Clearing the field now persistsbuttonUrl === ''instead of running the normalization path.Also applies to: 572-577
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/koenig-lexical/src/components/ui/cards/HeaderCard/v2/HeaderCard.tsx` around lines 109 - 111, The component is discarding the passed blur handler (handleButtonUrlBlur) and never forwarding it to InputUrlSetting, causing clearing the field to persist an empty buttonUrl instead of running normalization; update HeaderCard (the function receiving HeaderCardV2Props) to forward the blur handler (use the received _handleButtonUrlBlur or rename to handleButtonUrlBlur) into the InputUrlSetting via its onBlur prop (e.g., onBlur={_handleButtonUrlBlur}), and remove any unused dead variable so the blur normalization path is executed; apply the same fix in the other occurrence where InputUrlSetting is rendered (the block referenced around the later occurrence).packages/koenig-lexical/src/components/ui/cards/FileCard.tsx-9-12 (1)
9-12:⚠️ Potential issue | 🟡 MinorDefault missing upload progress to
0.Line 11 now renders
width: "undefined%"until the first progress event arrives, so the uploading state starts with an invalid CSS width.💡 Proposed fix
function FileUploading({progress}: {progress?: number}) { + const safeProgress = progress ?? 0; const progressStyle = { - width: `${progress?.toFixed(0)}%` + width: `${safeProgress.toFixed(0)}%` };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/koenig-lexical/src/components/ui/cards/FileCard.tsx` around lines 9 - 12, FileUploading currently builds progressStyle.width from progress which can be undefined and yields "undefined%"; change the construction in the FileUploading component so progress defaults to 0 (e.g. use progress ?? 0 or Number.isFinite check) before calling toFixed, ensuring progressStyle (and the width string) is always valid; update references in the FileUploading function where progress and progressStyle are used.packages/koenig-lexical/src/components/ui/BackgroundImagePicker.tsx-6-13 (1)
6-13:⚠️ Potential issue | 🟡 MinorDefault missing upload progress to
0.Line 12 has the same
undefined%width issue as the file card: if upload starts beforeprogressis populated, the bar renders with an invalid width.💡 Proposed fix
function FileUploading({progress}: FileUploadingProps) { + const safeProgress = progress ?? 0; const progressStyle = { - width: `${progress?.toFixed(0)}%` + width: `${safeProgress.toFixed(0)}%` };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/koenig-lexical/src/components/ui/BackgroundImagePicker.tsx` around lines 6 - 13, FileUploading can render an invalid width when progress is undefined; ensure progress defaults to 0 before formatting so the width is always valid. Update the FileUploading component to coerce the incoming progress (e.g., using progress ?? 0 or Number(progress) with a fallback) and use that value when calling toFixed and building progressStyle (refer to the FileUploading function and its progress prop) so the width string is always like "0%".
| const editor = editorAPI.editorInstance as {_rootElement: HTMLElement; getEditorState: () => {read: (fn: () => void) => void}}; | ||
|
|
||
| // if a mousedown and subsequent mouseup occurs below the editor | ||
| // canvas, focus the editor and put the cursor at the end of the document | ||
| let {bottom} = editor._rootElement.getBoundingClientRect(); | ||
| const {bottom} = editor._rootElement.getBoundingClientRect(); | ||
| if (event.pageY > bottom && event.clientY > bottom) { |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
For the current Lexical version, is _rootElementan internal/private field, and isgetRootElement() the supported public API for obtaining the editor root element?
💡 Result:
The current Lexical version is approximately 0.42.0 (latest release v0.42.0 on 2026-03-19). Yes, _rootElement is an internal/private field on LexicalEditor (prefixed with underscore, listed as internal property in type signatures). Yes, getRootElement is the supported public API method for obtaining the editor root element, documented in official API reference with guidance to use registerRootListener for stable event handling as the direct reference may not be stable.
Citations:
- 1: https://www.npmjs.com/package/lexical?activeTab=versions
- 2: https://github.com/facebook/lexical
- 3: https://lexical.dev/docs/api/classes/lexical.LexicalEditor
- 4: https://lexical.dev/docs/packages/lexical
- 5: https://lexical.dev/docs/getting-started/quick-start
- 6: https://www.mintlify.com/facebook/lexical/concepts/editor
Replace private _rootElement with public getRootElement() API.
Lexical's _rootElement is an internal field that may break on library updates. Use the public getRootElement() method instead, which is the documented API for accessing the editor root.
Suggested fix
-const editor = editorAPI.editorInstance as {_rootElement: HTMLElement; getEditorState: () => {read: (fn: () => void) => void}};
+const editor = editorAPI.editorInstance as {
+ getRootElement?: () => HTMLElement | null;
+ getEditorState: () => {read: (fn: () => void) => void};
+};
+const rootElement = editor.getRootElement?.();
+if (!rootElement) {
+ skipFocusEditor.current = false;
+ return;
+}
-const {bottom} = editor._rootElement.getBoundingClientRect();
+const {bottom} = rootElement.getBoundingClientRect();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/koenig-lexical/demo/DemoApp.tsx` around lines 297 - 302, Replace the
use of the internal field _rootElement on editorAPI.editorInstance with the
public API getRootElement(): locate the code where editor is assigned from
editorAPI.editorInstance and currently accesses
editor._rootElement.getBoundingClientRect(), then call editor.getRootElement()
and use its getBoundingClientRect() result instead (keep the existing logic
comparing event.pageY and event.clientY to bottom). Ensure you update references
to _rootElement throughout this block to use getRootElement().
| function saveContent() { | ||
| const serializedState = editorAPI.serialize(); | ||
| const serializedState = editorAPI!.serialize(); | ||
| const encodedContent = encodeURIComponent(serializedState); | ||
| searchParams.set('content', encodedContent); | ||
| setSearchParams(searchParams); | ||
| } |
There was a problem hiding this comment.
Guard saveContent before serializing editor state.
editorAPI!.serialize() can throw if save is triggered before API registration completes.
Proposed fix
function saveContent() {
- const serializedState = editorAPI!.serialize();
+ if (!editorAPI) {
+ return;
+ }
+ const serializedState = editorAPI.serialize();
const encodedContent = encodeURIComponent(serializedState);
searchParams.set('content', encodedContent);
setSearchParams(searchParams);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function saveContent() { | |
| const serializedState = editorAPI.serialize(); | |
| const serializedState = editorAPI!.serialize(); | |
| const encodedContent = encodeURIComponent(serializedState); | |
| searchParams.set('content', encodedContent); | |
| setSearchParams(searchParams); | |
| } | |
| function saveContent() { | |
| if (!editorAPI) { | |
| return; | |
| } | |
| const serializedState = editorAPI.serialize(); | |
| const encodedContent = encodeURIComponent(serializedState); | |
| searchParams.set('content', encodedContent); | |
| setSearchParams(searchParams); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/koenig-lexical/demo/DemoApp.tsx` around lines 343 - 348, saveContent
currently calls editorAPI!.serialize() without checking that editorAPI is ready;
guard the call by verifying editorAPI is defined and has serialize before using
it (e.g., return early or no-op if editorAPI is falsy), and wrap the serialize
call in a try/catch to avoid uncaught exceptions; update the function that
references saveContent, the editorAPI variable, and the
setSearchParams/searchParams logic to only set params when serialization
succeeds.
| > | ||
| <CaptionPlugin parentEditor={parentEditor} /> | ||
| <RestrictContentPlugin paragraphs={paragraphs} /> | ||
| <RestrictContentPlugin allowBr={false} paragraphs={paragraphs} /> |
There was a problem hiding this comment.
allowBr={false} regresses caption line breaks.
This file still explicitly returns false on Shift+Enter so the caption editor can insert a line break on Lines 85-88. Disabling <br> here blocks that path, so multi-line captions stop working.
🩹 Proposed fix
- <RestrictContentPlugin allowBr={false} paragraphs={paragraphs} />
+ <RestrictContentPlugin paragraphs={paragraphs} />📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <RestrictContentPlugin allowBr={false} paragraphs={paragraphs} /> | |
| <RestrictContentPlugin paragraphs={paragraphs} /> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/koenig-lexical/src/components/KoenigCaptionEditor.tsx` at line 159,
The RestrictContentPlugin is explicitly preventing <br> insertion by passing
allowBr={false}, which conflicts with the Shift+Enter handler in this component
that intentionally returns false to allow line breaks; remove the
allowBr={false} prop (or set allowBr={true}) from the <RestrictContentPlugin>
usage in KoenigCaptionEditor so the existing Shift+Enter logic (the handler that
returns false to permit breaks) can resume inserting line breaks; keep the
paragraphs prop as-is.
| const _onChange = React.useCallback((editorState: EditorState) => { | ||
| if (sharedOnChange) { | ||
| // sharedOnChange is called for the main editor and nested editors, we want to | ||
| // make sure we don't accidentally serialize only the contents of the nested | ||
| // editor so we need to use the parent editor when it exists | ||
| const primaryEditorState = (editor._parentEditor || editor).getEditorState(); | ||
| const json = primaryEditorState.toJSON(); | ||
| sharedOnChange(json); | ||
| sharedOnChange(json as unknown as import('lexical').EditorState, editor, new Set<string>()); | ||
| } |
There was a problem hiding this comment.
Pass the real EditorState and update tags to sharedOnChange.
Lines 87-88 serialize the state to JSON, cast it back to EditorState, and replace the real tag set with new Set<string>(). Any shared handler that calls read()/toJSON() or checks tags will get the wrong runtime object and metadata.
🩹 Proposed fix
- const _onChange = React.useCallback((editorState: EditorState) => {
+ const _onChange = React.useCallback((editorState: EditorState, _nextEditor: unknown, tags: Set<string>) => {
if (sharedOnChange) {
// sharedOnChange is called for the main editor and nested editors, we want to
// make sure we don't accidentally serialize only the contents of the nested
// editor so we need to use the parent editor when it exists
const primaryEditorState = (editor._parentEditor || editor).getEditorState();
- const json = primaryEditorState.toJSON();
- sharedOnChange(json as unknown as import('lexical').EditorState, editor, new Set<string>());
+ sharedOnChange(primaryEditorState, editor, tags);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const _onChange = React.useCallback((editorState: EditorState) => { | |
| if (sharedOnChange) { | |
| // sharedOnChange is called for the main editor and nested editors, we want to | |
| // make sure we don't accidentally serialize only the contents of the nested | |
| // editor so we need to use the parent editor when it exists | |
| const primaryEditorState = (editor._parentEditor || editor).getEditorState(); | |
| const json = primaryEditorState.toJSON(); | |
| sharedOnChange(json); | |
| sharedOnChange(json as unknown as import('lexical').EditorState, editor, new Set<string>()); | |
| } | |
| const _onChange = React.useCallback((editorState: EditorState, _nextEditor: unknown, tags: Set<string>) => { | |
| if (sharedOnChange) { | |
| // sharedOnChange is called for the main editor and nested editors, we want to | |
| // make sure we don't accidentally serialize only the contents of the nested | |
| // editor so we need to use the parent editor when it exists | |
| const primaryEditorState = (editor._parentEditor || editor).getEditorState(); | |
| sharedOnChange(primaryEditorState, editor, tags); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/koenig-lexical/src/components/KoenigComposableEditor.tsx` around
lines 81 - 89, The _onChange callback currently serializes primaryEditorState to
JSON and casts it back before calling sharedOnChange, and it also passes a fresh
empty tag set; instead pass the actual EditorState instance and the real tag set
so consumers calling read()/toJSON() or inspecting tags see the real runtime
objects—locate _onChange, the computation of primaryEditorState
((editor._parentEditor || editor).getEditorState()), and replace the json cast
and new Set<string>() arguments in the sharedOnChange(...) call with the actual
primaryEditorState (or its EditorState instance) and the editor's real tag set.
| {!isCollabActive && <HistoryPlugin externalHistoryState={historyState} />} {/* adds undo/redo, in multiplayer that's handled by yjs */} | ||
| <KoenigBehaviourPlugin containerElem={editorContainerRef} cursorDidExitAtTop={cursorDidExitAtTop} isNested={isNested} /> | ||
| <MarkdownShortcutPlugin transformers={markdownTransformers} /> | ||
| { } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Does TSX/JSX allow an empty expression container written as {} inside component markup?
💡 Result:
No, TSX/JSX does not allow an empty expression container written as {} inside component markup. It results in a syntax error. JSX expression containers {} must contain valid JavaScript expressions. An empty {} is parsed as an empty JSXExpressionContainer with a JSXEmptyExpression, but this is invalid in JSX children and attributes because JSX requires non-empty expressions that evaluate to valid React children or prop values. Official React documentation states that JSX expressions must contain JavaScript expressions, and empty {} in attributes produces invalid JavaScript after transpilation (e.g., prop={} becomes invalid object syntax). TypeScript explicitly handles empty JSX expressions as non-semantically important but still flags them in certain contexts like required children. Babel and TypeScript parsers recognize JSXEmptyExpression for AST purposes (e.g., comments), but it is not allowed as a standalone child in markup. Stack Overflow examples confirm "Expression expected" errors in TSX/JSX for empty {} or statements inside {}. Valid alternatives for no content: {null}, {false}, {undefined}, or {''} (empty string, renders nothing visible).
Citations:
- 1: https://stackoverflow.com/questions/44014344/jsx-attributes-must-only-be-assigned-a-non-empty-expression
- 2: Do not consider empty jsx expressions semantically important children microsoft/TypeScript#41166
- 3: types: JSXExpressionContainer does not allow
expressionto be of typeJSXEmptyExpressionbabel/babel#8549 - 4: https://reactjs.org/docs/jsx-in-depth.html
- 5: https://stackoverflow.com/questions/56371020/how-do-i-fix-expression-expected-ts1109-in-vscode
- 6: https://stackoverflow.com/questions/68932653/expression-expected-and-syntax-error-unexpected-token-jsx-nextjs
- 7: https://stackoverflow.com/questions/59142974/react-typescript-expected-ts1005
- 8: https://stackoverflow.com/questions/73250046/why-does-jsx-parse-empty-spaces-as-children
- 9: why JSX attributes must only be assigned a non-empty expression ? facebook/react#13670
🏁 Script executed:
cat -n packages/koenig-lexical/src/components/KoenigComposableEditor.tsx | sed -n '130,140p'Repository: TryGhost/Koenig
Length of output: 1092
Remove the empty JSX expression container at line 135.
The bare { } is invalid TSX/JSX syntax and will cause a parsing error. Empty expression containers are not allowed in JSX; valid alternatives are {null}, {false}, or {undefined} if conditional rendering is needed, but in this case the line should simply be removed.
🩹 Proposed fix
- { }
{floatingAnchorElem && (<FloatingToolbarPlugin anchorElem={floatingAnchorElem} hiddenFormats={hiddenFormats} isSnippetsEnabled={isSnippetsEnabled} />)}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| { } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/koenig-lexical/src/components/KoenigComposableEditor.tsx` at line
135, The JSX contains an empty expression container "{ }" inside the
KoenigComposableEditor render which is invalid and causes a TSX parse error;
remove that empty JSX expression from the component (or replace it with a
deliberate placeholder like {null}/{false}/{undefined} if conditional rendering
was intended) — locate the empty container in the KoenigComposableEditor
component render and delete it so the JSX is valid.
| const handleImageChange = async (files: File[] | FileList | null) => { | ||
| // reset original src so it can be replaced with preview and upload progress | ||
| editor.update(() => { | ||
| const node = $getNodeByKey(nodeKey); | ||
| const node = $getHeaderNodeByKey(nodeKey); | ||
| if (!node) {return;} | ||
| node.backgroundImageSrc = ''; | ||
| }); | ||
|
|
||
| const {imageSrc, width, height} = await backgroundImageUploadHandler(files, imageUploader.upload); | ||
| const result = await backgroundImageUploadHandler(Array.from(files!), imageUploader.upload); | ||
| if (!result) {return;} | ||
| const {imageSrc, width, height} = result; | ||
|
|
||
| editor.update(() => { | ||
| const node = $getNodeByKey(nodeKey); | ||
| node.backgroundImageSrc = imageSrc; | ||
| const node = $getHeaderNodeByKey(nodeKey); | ||
| if (!node) {return;} | ||
| node.backgroundImageSrc = imageSrc ?? ""; | ||
| node.backgroundImageWidth = width; | ||
| node.backgroundImageHeight = height; | ||
| }); | ||
|
|
||
| setLastBackgroundImage(imageSrc); | ||
| setLastBackgroundImage(imageSrc as string); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
sed -n '143,163p' packages/koenig-lexical/src/nodes/header/v2/HeaderNodeComponent.tsxRepository: TryGhost/Koenig
Length of output: 907
Guard cancelled image picks before clearing the node.
This handler accepts null, but it clears backgroundImageSrc and then calls Array.from(files!) without first checking if files is null. Cancelling the file picker will wipe the current image and throw a runtime error. Check for null/empty input first, then normalize imageSrc once to eliminate redundant type casting.
🔧 Suggested fix
const handleImageChange = async (files: File[] | FileList | null) => {
+ if (!files || files.length === 0) {
+ return;
+ }
+
// reset original src so it can be replaced with preview and upload progress
editor.update(() => {
const node = $getHeaderNodeByKey(nodeKey);
if (!node) {return;}
node.backgroundImageSrc = '';
});
- const result = await backgroundImageUploadHandler(Array.from(files!), imageUploader.upload);
+ const result = await backgroundImageUploadHandler(Array.from(files), imageUploader.upload);
if (!result) {return;}
- const {imageSrc, width, height} = result;
+ const {width, height} = result;
+ const imageSrc = result.imageSrc ?? '';
editor.update(() => {
const node = $getHeaderNodeByKey(nodeKey);
if (!node) {return;}
- node.backgroundImageSrc = imageSrc ?? "";
+ node.backgroundImageSrc = imageSrc;
node.backgroundImageWidth = width;
node.backgroundImageHeight = height;
});
- setLastBackgroundImage(imageSrc as string);
+ setLastBackgroundImage(imageSrc);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const handleImageChange = async (files: File[] | FileList | null) => { | |
| // reset original src so it can be replaced with preview and upload progress | |
| editor.update(() => { | |
| const node = $getNodeByKey(nodeKey); | |
| const node = $getHeaderNodeByKey(nodeKey); | |
| if (!node) {return;} | |
| node.backgroundImageSrc = ''; | |
| }); | |
| const {imageSrc, width, height} = await backgroundImageUploadHandler(files, imageUploader.upload); | |
| const result = await backgroundImageUploadHandler(Array.from(files!), imageUploader.upload); | |
| if (!result) {return;} | |
| const {imageSrc, width, height} = result; | |
| editor.update(() => { | |
| const node = $getNodeByKey(nodeKey); | |
| node.backgroundImageSrc = imageSrc; | |
| const node = $getHeaderNodeByKey(nodeKey); | |
| if (!node) {return;} | |
| node.backgroundImageSrc = imageSrc ?? ""; | |
| node.backgroundImageWidth = width; | |
| node.backgroundImageHeight = height; | |
| }); | |
| setLastBackgroundImage(imageSrc); | |
| setLastBackgroundImage(imageSrc as string); | |
| const handleImageChange = async (files: File[] | FileList | null) => { | |
| if (!files || files.length === 0) { | |
| return; | |
| } | |
| // reset original src so it can be replaced with preview and upload progress | |
| editor.update(() => { | |
| const node = $getHeaderNodeByKey(nodeKey); | |
| if (!node) {return;} | |
| node.backgroundImageSrc = ''; | |
| }); | |
| const result = await backgroundImageUploadHandler(Array.from(files), imageUploader.upload); | |
| if (!result) {return;} | |
| const {width, height} = result; | |
| const imageSrc = result.imageSrc ?? ''; | |
| editor.update(() => { | |
| const node = $getHeaderNodeByKey(nodeKey); | |
| if (!node) {return;} | |
| node.backgroundImageSrc = imageSrc; | |
| node.backgroundImageWidth = width; | |
| node.backgroundImageHeight = height; | |
| }); | |
| setLastBackgroundImage(imageSrc); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/koenig-lexical/src/nodes/header/v2/HeaderNodeComponent.tsx` around
lines 143 - 163, The handler handleImageChange currently clears the node and
then does Array.from(files!) which will crash if files is null; first return
early if files is null or has length 0 before calling editor.update or
backgroundImageUploadHandler, then call
backgroundImageUploadHandler(Array.from(files), imageUploader.upload) without
non-null assertions, capture result.imageSrc once into a local normalized
string, and only after a successful result perform editor.update (using
$getHeaderNodeByKey) to set backgroundImageSrc/width/height and call
setLastBackgroundImage(imageSrc) — this preserves the existing image on cancel
and removes redundant type casting.
| openImageEditor={openImageEditor as (opts: unknown) => void} | ||
| renderLabels={!!cardConfig.renderLabels} | ||
| setFileInputRef={ref => fileInputRef.current = ref} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect how SignupCard types and consumes these props.
fd 'SignupCard\.(tsx|jsx|ts|js)$' packages/koenig-lexical/src | while read -r f; do
echo "== $f =="
rg -n -C2 'openImageEditor|setFileInputRef|ref=' "$f"
doneRepository: TryGhost/Koenig
Length of output: 1816
🏁 Script executed:
# Inspect SignupNodeComponent.tsx around lines 305-307 to see the full context
fd 'SignupNodeComponent\.(tsx|jsx|ts|js)$' packages/koenig-lexical/src | head -1 | xargs -I {} sed -n '295,320p' {} | cat -nRepository: TryGhost/Koenig
Length of output: 1461
🏁 Script executed:
# Also check the declaration of openImageEditor and fileInputRef in SignupNodeComponent.tsx
fd 'SignupNodeComponent\.(tsx|jsx|ts|js)$' packages/koenig-lexical/src | head -1 | xargs -I {} rg -n 'openImageEditor|fileInputRef|const.*=.*useRef' {} | head -30Repository: TryGhost/Koenig
Length of output: 457
🏁 Script executed:
# Check the full signature/props of SignupNodeComponent to understand where openImageEditor comes from
fd 'SignupNodeComponent\.(tsx|jsx|ts|js)$' packages/koenig-lexical/src | head -1 | xargs -I {} head -100 {}Repository: TryGhost/Koenig
Length of output: 3875
🏁 Script executed:
# Find and inspect the usePinturaEditor hook
fd 'usePinturaEditor\.(tsx|jsx|ts|js)$' packages/koenig-lexical/src | head -1 | xargs -I {} cat -n {}Repository: TryGhost/Koenig
Length of output: 7320
🏁 Script executed:
# Also search for usePinturaEditor implementation if not found by filename
rg -n 'export.*usePinturaEditor|function usePinturaEditor|const usePinturaEditor' packages/koenig-lexical/src -A 30Repository: TryGhost/Koenig
Length of output: 2760
Use a block-bodied ref callback and clarify the callback prop boundary.
openImageEditor as (opts: unknown) => void casts away the real signature: {image: string; handleSave: (blob: Blob) => void}. The ref callback ref => fileInputRef.current = ref implicitly returns the assigned element instead of void. SignupCard expects void, and in React 19, a non-void return from a ref callback is treated as a cleanup function, causing unintended behavior.
Change the ref callback to a block body: ref => { fileInputRef.current = ref; } or wrap it in useCallback. Update openImageEditor's type in SignupCard props to match its actual signature rather than using unknown.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/koenig-lexical/src/nodes/SignupNodeComponent.tsx` around lines 305 -
307, The SignupCard props are using a loose cast and a concise ref callback that
can return a value: replace the openImageEditor prop cast openImageEditor as
(opts: unknown) => void with the real function signature used by SignupCard
(e.g. openImageEditor: (args: {image: string; handleSave: (blob: Blob) => void})
=> void) in the SignupCard props/type definitions, and change the
setFileInputRef callback to a block-bodied callback (or use useCallback) so it
returns void (e.g. ref => { fileInputRef.current = ref; }) to avoid React 19
treating the returned element as a cleanup function; update the SignupCard prop
type for openImageEditor and the component callsite to match the concrete
signature and ensure setFileInputRef is typed as (el: HTMLInputElement | null)
=> void.
| const handleCustomThumbnailChange = async (files: File[] | FileList) => { | ||
| const customThumbnailUploadResult = await customThumbnailUploader.upload(Array.from(files)); | ||
| const imageUrl = customThumbnailUploadResult?.[0]?.url; | ||
| const {width, height} = await getImageDimensions(imageUrl); | ||
| const {width, height} = await getImageDimensions(imageUrl!); | ||
|
|
||
| if (imageUrl) { | ||
| editor.update(() => { | ||
| const node = $getNodeByKey(nodeKey); | ||
| const node = $getVideoNodeByKey(nodeKey); | ||
| if (!node) {return;} | ||
| node.customThumbnailSrc = imageUrl; | ||
| node.thumbnailWidth = width; | ||
| node.thumbnailHeight = height; | ||
| }); | ||
| } | ||
| }; |
There was a problem hiding this comment.
Non-null assertion on potentially undefined imageUrl could cause runtime error.
getImageDimensions(imageUrl!) is called before checking if imageUrl exists. If the upload fails and returns undefined, this will pass undefined to getImageDimensions, which could throw or behave unexpectedly.
Proposed fix
const handleCustomThumbnailChange = async (files: File[] | FileList) => {
const customThumbnailUploadResult = await customThumbnailUploader.upload(Array.from(files));
const imageUrl = customThumbnailUploadResult?.[0]?.url;
- const {width, height} = await getImageDimensions(imageUrl!);
- if (imageUrl) {
+ if (!imageUrl) {
+ return;
+ }
+
+ const {width, height} = await getImageDimensions(imageUrl);
+
+ editor.update(() => {
+ const node = $getVideoNodeByKey(nodeKey);
+ if (!node) {return;}
+ node.customThumbnailSrc = imageUrl;
+ node.thumbnailWidth = width;
+ node.thumbnailHeight = height;
+ });
- editor.update(() => {
- const node = $getVideoNodeByKey(nodeKey);
- if (!node) {return;}
- node.customThumbnailSrc = imageUrl;
- node.thumbnailWidth = width;
- node.thumbnailHeight = height;
- });
- }
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const handleCustomThumbnailChange = async (files: File[] | FileList) => { | |
| const customThumbnailUploadResult = await customThumbnailUploader.upload(Array.from(files)); | |
| const imageUrl = customThumbnailUploadResult?.[0]?.url; | |
| const {width, height} = await getImageDimensions(imageUrl); | |
| const {width, height} = await getImageDimensions(imageUrl!); | |
| if (imageUrl) { | |
| editor.update(() => { | |
| const node = $getNodeByKey(nodeKey); | |
| const node = $getVideoNodeByKey(nodeKey); | |
| if (!node) {return;} | |
| node.customThumbnailSrc = imageUrl; | |
| node.thumbnailWidth = width; | |
| node.thumbnailHeight = height; | |
| }); | |
| } | |
| }; | |
| const handleCustomThumbnailChange = async (files: File[] | FileList) => { | |
| const customThumbnailUploadResult = await customThumbnailUploader.upload(Array.from(files)); | |
| const imageUrl = customThumbnailUploadResult?.[0]?.url; | |
| if (!imageUrl) { | |
| return; | |
| } | |
| const {width, height} = await getImageDimensions(imageUrl); | |
| editor.update(() => { | |
| const node = $getVideoNodeByKey(nodeKey); | |
| if (!node) {return;} | |
| node.customThumbnailSrc = imageUrl; | |
| node.thumbnailWidth = width; | |
| node.thumbnailHeight = height; | |
| }); | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/koenig-lexical/src/nodes/VideoNodeComponent.tsx` around lines 146 -
160, The handler handleCustomThumbnailChange currently calls
getImageDimensions(imageUrl!) before verifying imageUrl; change the flow to
first check the upload result (from customThumbnailUploader.upload) and ensure
imageUrl is defined, returning early or handling the error if undefined, then
call getImageDimensions(imageUrl) only when imageUrl exists; after obtaining
width/height, perform editor.update and $getVideoNodeByKey(nodeKey) mutation to
set node.customThumbnailSrc, node.thumbnailWidth and node.thumbnailHeight as
before.
| const linkNode = $createLinkNode(item.value!); | ||
| const textNode = $createTextNode(item.label ?? ''); | ||
| linkNode.append(textNode); | ||
| linkNode.setFormat(focusedAtLinkNode.getLinkFormat()); | ||
| linkNode.setFormat(focusedAtLinkNode.getLinkFormat() as unknown as ElementFormatType); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In the current Lexical API, what types do ElementNode.setFormatandTextNode.setFormataccept, and where should inline bold/italic formatting be applied when creating aLinkNodewith a childTextNode?
💡 Result:
In the current Lexical API: - TextNode.setFormat(format) accepts a number: either a TextFormatType (specifying one format, which clears others) or a 32-bit integer representing combined TextFormatTypes (e.g., bold, italic). Use toggleFormat for toggling. Common formats: bold, italic, underline, strikethrough, code, subscript, superscript. - ElementNode.setFormat(format) accepts a number representing ElementFormatType (e.g., alignment like left, center, right, justify). Inline bold/italic formatting (text formats) should be applied to the child TextNode within the LinkNode, not the LinkNode itself (which is an inline ElementNode using element formats). Example: const linkNode = $createLinkNode('https://example.com'); const textNode = $createTextNode('Bold italic link').toggleFormat(FORMAT_BOLD).toggleFormat(FORMAT_ITALIC); linkNode.append(textNode); LinkNode supports children and text formatting on them works normally per docs and node hierarchy.
Citations:
- 1: https://lexical.dev/docs/api/classes/lexical.ElementNode
- 2: https://lexical.dev/docs/api/classes/lexical_link.LinkNode
- 3: https://www.mintlify.com/facebook/lexical/concepts/nodes
- 4: https://lexical.dev/docs/api/classes/lexical_code.CodeHighlightNode
- 5: https://lexical.dev/docs/api/classes/lexical.TextNode
- 6: ⏸ TextNode setFormat/setDetail to accept a number only facebook/lexical#3038
- 7: https://lexical.dev/docs/concepts/nodes
Apply text formatting to textNode, not LinkNode.
When focusedAtLinkNode.getLinkFormat() returns a text-format bitmask (bold/italic flags), passing it to LinkNode.setFormat() with an unsafe cast violates the Lexical API contract. LinkNode inherits ElementNode, which expects ElementFormatType (alignment values), while TextNode.setFormat() accepts text format bitmasks. Bold and italic formatting on the label will not be preserved. Move the format to the child textNode before appending:
Suggested fix
const linkNode = $createLinkNode(item.value!);
const textNode = $createTextNode(item.label ?? '');
+ textNode.setFormat(focusedAtLinkNode.getLinkFormat() ?? 0);
linkNode.append(textNode);
- linkNode.setFormat(focusedAtLinkNode.getLinkFormat() as unknown as ElementFormatType);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/koenig-lexical/src/plugins/AtLinkPlugin.tsx` around lines 443 - 446,
The code incorrectly applies a text-format bitmask to the LinkNode (via
linkNode.setFormat(...)) which expects element/alignment formats; instead, read
the text-format from focusedAtLinkNode.getLinkFormat() and apply it to the child
TextNode (textNode.setFormat(...)) before appending to the link node; update the
logic around $createLinkNode, $createTextNode, textNode.append/ linkNode.append
so the text formatting is set on textNode (not on LinkNode) and remove the
unsafe cast to ElementFormatType.
| if (draggableInfo.type === 'image') { | ||
| const targetNode = $getNearestNodeFromDOMNode(droppables[draggableInfo.insertIndex]); | ||
| const imageNode = $createImageNode(draggableInfo.dataset); | ||
| targetNode.insertBefore(imageNode); | ||
| const targetNode = $getNearestNodeFromDOMNode(droppables[draggableInfo.insertIndex ?? 0] as Node); | ||
| const imageNode = $createImageNode((draggableInfo.dataset as Record<string, unknown>) ?? {}); | ||
| targetNode?.insertBefore(imageNode); |
There was a problem hiding this comment.
Handle image drops at the end of the document.
When the drop indicator is below the last block, insertIndex becomes droppables.length. Line 180 then indexes past the array, so this branch can return success without inserting the image.
🩹 Proposed fix
if (draggableInfo.type === 'image') {
- const targetNode = $getNearestNodeFromDOMNode(droppables[draggableInfo.insertIndex ?? 0] as Node);
const imageNode = $createImageNode((draggableInfo.dataset as Record<string, unknown>) ?? {});
- targetNode?.insertBefore(imageNode);
+ if ((draggableInfo.insertIndex ?? 0) >= droppables.length) {
+ const targetNode = $getNearestNodeFromDOMNode(droppables[droppables.length - 1] as Node);
+ targetNode?.insertAfter(imageNode);
+ } else {
+ const targetNode = $getNearestNodeFromDOMNode(droppables[draggableInfo.insertIndex ?? 0] as Node);
+ targetNode?.insertBefore(imageNode);
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (draggableInfo.type === 'image') { | |
| const targetNode = $getNearestNodeFromDOMNode(droppables[draggableInfo.insertIndex]); | |
| const imageNode = $createImageNode(draggableInfo.dataset); | |
| targetNode.insertBefore(imageNode); | |
| const targetNode = $getNearestNodeFromDOMNode(droppables[draggableInfo.insertIndex ?? 0] as Node); | |
| const imageNode = $createImageNode((draggableInfo.dataset as Record<string, unknown>) ?? {}); | |
| targetNode?.insertBefore(imageNode); | |
| if (draggableInfo.type === 'image') { | |
| const imageNode = $createImageNode((draggableInfo.dataset as Record<string, unknown>) ?? {}); | |
| if ((draggableInfo.insertIndex ?? 0) >= droppables.length) { | |
| const targetNode = $getNearestNodeFromDOMNode(droppables[droppables.length - 1] as Node); | |
| targetNode?.insertAfter(imageNode); | |
| } else { | |
| const targetNode = $getNearestNodeFromDOMNode(droppables[draggableInfo.insertIndex ?? 0] as Node); | |
| targetNode?.insertBefore(imageNode); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/koenig-lexical/src/plugins/DragDropReorderPlugin.tsx` around lines
179 - 182, The image-drop branch can index past droppables when
draggableInfo.insertIndex === droppables.length (drop below the last block);
modify the branch in DragDropReorderPlugin (the block using
$getNearestNodeFromDOMNode, droppables, $createImageNode and
targetNode?.insertBefore) to handle that case by treating an out-of-range
insertIndex as “append to end”: if insertIndex >= droppables.length, resolve a
target node representing the document end (e.g., the last droppable's nearest
node or the root/parent block) and insert the newly created image node after
that node or append it to the root instead of indexing droppables[insertIndex];
otherwise keep the existing behavior.
e7606e5 to
af866fd
Compare
kevinansfield
force-pushed
the
ts/koenig-lexical-cleanup
branch
from
af866fd to
a386172
Compare
- Create tsconfig.json (strict, bundler moduleResolution, noEmit) - Rename all 328 source files (.js→.ts, .jsx→.tsx) - Rename all 71 test files (.js→.ts, .jsx→.tsx) - Add vite-env.d.ts for SVG and Vite type declarations - Rename vite.config.js/demo.js → .ts, playwright.config.js → .ts - Add type annotations to ~163 files (context, hooks, utils, components) - Replace .eslintrc.cjs with eslint.config.js (flat config) - Add typecheck script to package.json - tsc --noEmit passes with 0 errors Note: 165 files still have @ts-nocheck and need proper typing in a follow-up (PropTypes→interfaces, Lexical node types, etc.)
kevinansfield
force-pushed
the
ts/koenig-lexical-cleanup
branch
from
a386172 to
0fc7c7b
Compare
There was a problem hiding this comment.
Actionable comments posted: 19
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (12)
packages/koenig-lexical/src/components/ui/cards/MarkdownCard/MarkdownEditor.tsx (1)
165-171:⚠️ Potential issue | 🟠 MajorGuard
extraKeysbefore mutation to prevent runtime crashes.At Line 165,
getOption('extraKeys')is force-cast and then mutated. If it isnull/undefined/non-object, Lines 167–171 will throw.Proposed fix
- const keys = codemirror.getOption('extraKeys') as Record<string, unknown>; + const rawKeys = codemirror.getOption('extraKeys'); + const keys: Record<string, unknown> = + rawKeys && typeof rawKeys === 'object' + ? {...(rawKeys as Record<string, unknown>)} + : {};🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/koenig-lexical/src/components/ui/cards/MarkdownCard/MarkdownEditor.tsx` around lines 165 - 171, The code force-casts and mutates codemirror.getOption('extraKeys') which can be null/undefined or non-object; guard and normalize it first: retrieve extraKeys from codemirror.getOption('extraKeys'), if it's falsy or not an object create an empty object, then assign keys[shortcuts.toggleSpellcheck]=toggleSpellcheck and keys[shortcuts.openImageDialog]=openImageUploadDialog and, if unsplashConf, keys[shortcuts.openUnsplashDialog]=openUnsplashDialog, and finally write it back via codemirror.setOption('extraKeys', keys) so mutations are safe (refer to codemirror, shortcuts, toggleSpellcheck, openImageUploadDialog, openUnsplashDialog, unsplashConf).packages/koenig-lexical/src/components/DesignSandbox.tsx (1)
148-159:⚠️ Potential issue | 🟡 MinorAccessibility:
role="button"on a<div>requires keyboard support.The
<div>withrole="button"is not keyboard accessible (notabIndex, noonKeyDownhandler). This is a design sandbox so impact is limited, but the pattern could be copied elsewhere.♿ Suggested fix for accessibility
<div className="flex size-9 items-center justify-center" role="button" + tabIndex={0} + onKeyDown={(e) => { if (e.key === 'Enter' || e.key === ' ') { /* handle click */ } }} >Alternatively, consider using a
<button>element directly for native keyboard support.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/koenig-lexical/src/components/DesignSandbox.tsx` around lines 148 - 159, The ToolbarItem component uses a <div> with role="button" but lacks keyboard support; update ToolbarItem to make the control keyboard-accessible by either replacing the <div> (the element with className "flex size-9 items-center justify-center" inside ToolbarItem) with a native <button> or, if you must keep a div, add tabIndex={0}, an onKeyDown handler that triggers the same action on Enter and Space, and appropriate aria-label/aria-pressed as needed; ensure the Icon still receives className "fill-white" and any existing props passed to ToolbarItem are applied to the interactive element so keyboard and assistive tech can interact correctly.packages/koenig-lexical/demo/utils/useFileUpload.ts (2)
42-55:⚠️ Potential issue | 🟠 MajorGuard unknown upload
typebefore dereferencing config.
useFileUploadFndefaultstypeto'', butdefaultValidatorreadsfileTypes[type].extensionsunguarded. That can throw at runtime instead of returning a validation error.Proposed fix
- const extensions = fileTypes[type].extensions; + const typeConfig = fileTypes[type]; + if (!typeConfig) { + return `Unsupported upload type "${type}"`; + } + const extensions = typeConfig.extensions;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/koenig-lexical/demo/utils/useFileUpload.ts` around lines 42 - 55, The validator in useFileUpload's inner function useFileUploadFn (defaultValidator) dereferences fileTypes[type].extensions without guarding against an unknown or empty type, which can throw; update defaultValidator to first check that type is a valid key in fileTypes (and that fileTypes[type] and fileTypes[type].extensions exist), and if not return a validation error string (or false) instead of proceeding, so useFileUpload/useFileUploadFn and defaultValidator safely handle undefined or unsupported types.
137-142:⚠️ Potential issue | 🟠 MajorHandle
FileReaderfailure paths to avoid hanging uploads.The Promise resolves only on
load. If reading fails/aborts,awaitnever settles andisLoadingmay staytrue.Proposed fix
- const url = await new Promise<string | ArrayBuffer | null>((resolve) => { + const url = await new Promise<string | ArrayBuffer | null>((resolve, reject) => { reader.addEventListener('load', () => { resolve(reader.result); }, false); + reader.addEventListener('error', () => reject(reader.error), false); + reader.addEventListener('abort', () => reject(new Error('File read aborted')), false); reader.readAsDataURL(file); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/koenig-lexical/demo/utils/useFileUpload.ts` around lines 137 - 142, The Promise that reads the file in useFileUpload currently only resolves on the reader 'load' event and can hang if FileReader errors or is aborted; update the Promise around reader (the FileReader instance used in useFileUpload) to also handle 'error' and 'abort' events so the Promise always settles (resolve null or reject with the error), and remove/clean up all attached listeners in each handler to avoid leaks; ensure the calling code that sets isLoading handles the settled case (null/error) to clear isLoading and surface the failure appropriately.packages/koenig-lexical/src/components/ui/KeyboardSelectionWithGroups.tsx (1)
58-62:⚠️ Potential issue | 🟡 MinorGuard Enter selection against empty/non-selectable items.
onSelect(items[selectedIndex])can passundefinedwhen the list is empty (or index points to a non-selectable entry).Proposed fix
if (event.key === 'Enter') { event.preventDefault(); event.stopPropagation(); - onSelect(items[selectedIndex]); + const selectedItem = items[selectedIndex]; + if (selectedItem?.value) { + onSelect(selectedItem); + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/koenig-lexical/src/components/ui/KeyboardSelectionWithGroups.tsx` around lines 58 - 62, In KeyboardSelectionWithGroups' key handler (where it checks if event.key === 'Enter'), guard the Enter selection by validating that items[selectedIndex] exists and is selectable before calling onSelect: check that selectedIndex is within items.length and that the resolved item is not undefined (and if your items use a selectable/disabled property, ensure that property allows selection) and only then call onSelect(items[selectedIndex]); otherwise preventDefault/stopPropagation but do not invoke onSelect.packages/koenig-lexical/src/hooks/useSearchLinks.ts (1)
93-111:⚠️ Potential issue | 🟠 MajorReset
isSearchingon every async exit path.
setIsSearching(false)only runs on the happy path. A rejected search, a cancelled search returningundefined, or a switch from a pending search to a URL query can leave the hook stuck in the loading state. Wrap these awaits intry/finally, and gate late results with a request id before mutating state.Also applies to: 121-132
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/koenig-lexical/src/hooks/useSearchLinks.ts` around lines 93 - 111, The _search async function can leave setIsSearching(true) active on error/cancel/URL-branch; wrap the awaiting logic in a try/finally so setIsSearching(false) always runs, and add a simple request id (incrementing counter captured per-call) to gate state updates (setListOptions/convertSearchResultsToListOptions) so late or cancelled results (including when URL_QUERY_REGEX.test(term) is true) don't overwrite newer state; update references in _search, the URL branch, and the block that handles results (searchLinks, setListOptions, setIsSearching) to use these guards.packages/koenig-lexical/src/components/ui/cards/HeaderCard/v2/HeaderCard.tsx (1)
215-235:⚠️ Potential issue | 🟠 MajorKeep the chosen background even when
textColoris unset.
backgroundColoris required, butwrapperStyle()now falls through to the transparent checkerboard branch unlesstextColoris also set. Any render where text color is temporarily undefined will drop the configured card background.Suggested fix
- } else if (backgroundColor && textColor) { + } else if (backgroundColor) { return { backgroundColor: hexColorValue(backgroundColor), - color: hexColorValue(textColor) + ...(textColor ? {color: hexColorValue(textColor)} : {}) }; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/koenig-lexical/src/components/ui/cards/HeaderCard/v2/HeaderCard.tsx` around lines 215 - 235, wrapperStyle currently falls through to the transparent checkerboard when textColor is unset because the backgroundColor branch requires textColor; update wrapperStyle so backgroundImage handling and backgroundColor handling do not require textColor. Specifically, in the function wrapperStyle: keep the backgroundImage branch but remove textColor from its condition (use backgroundImageSrc && layout !== 'split'), and in the backgroundColor branch change the condition to check backgroundColor only (not backgroundColor && textColor) and set color only if textColor is present (e.g., include color: hexColorValue(textColor) conditionally). Ensure the final default checkerboard branch is only used when neither backgroundImageSrc nor backgroundColor are set.packages/koenig-lexical/src/components/ui/ImageUploadForm.tsx (1)
16-24:⚠️ Potential issue | 🟠 MajorMove the change handler to the file input, not the form.
The
onFileChangeprop is typed asChangeEvent<HTMLInputElement>, but casting it toReact.FormEventHandler<HTMLFormElement>masks a real type mismatch. WhenonChangefires on the form,currentTargetis the form itself, not the file input—any handler code accessingcurrentTarget.fileswill break at runtime. MoveonChangeonto the<input>element and remove the cast.Suggested fix
- <form onChange={onFileChange as unknown as React.FormEventHandler<HTMLFormElement>}> + <form> <input ref={fileInputRef} accept={accept} disabled={disabled} hidden={true} multiple={multiple} name="image-input" type='file' + onChange={onFileChange} onClick={e => e.stopPropagation()} /> </form>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/koenig-lexical/src/components/ui/ImageUploadForm.tsx` around lines 16 - 24, The onChange handler is currently attached to the <form> with an incorrect cast which makes currentTarget refer to the form instead of the input; move the onChange prop (onFileChange) from the form element to the <input> element (the one using fileInputRef, accept, multiple, name="image-input") and remove the cast to React.FormEventHandler<HTMLFormElement> so onFileChange remains typed as ChangeEvent<HTMLInputElement> and can safely access event.currentTarget.files; keep the form's other attributes and the input's ref and props unchanged.packages/koenig-lexical/src/components/ui/cards/VideoCard.tsx (2)
110-140:⚠️ Potential issue | 🟠 MajorMake the editable-path callbacks required when
isEditingis true.The props
onCardWidthChangeandonCustomThumbnailChangeare optional in the interface, but whenisEditingis true, the settings panel renders interactive controls that use these callbacks with non-null assertions (!). If these callbacks are omitted by the parent, TypeScript is silenced but the controls fail at runtime. Use a discriminated union type or guard this panel until both callbacks are provided.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/koenig-lexical/src/components/ui/cards/VideoCard.tsx` around lines 110 - 140, The settings panel renders interactive controls when isEditing is true but uses non-null assertions for onCardWidthChange and onCustomThumbnailChange (used by ButtonGroupSetting and MediaUploadSetting), which can crash if those callbacks are omitted; fix by making the props a discriminated union so that when isEditing: true the props type requires onCardWidthChange and onCustomThumbnailChange, or alternatively guard the panel render so it only shows when isEditing && onCardWidthChange && onCustomThumbnailChange are truthy, updating the VideoCard component and its props interface accordingly to reference the onCardWidthChange and onCustomThumbnailChange symbols (and ButtonGroupSetting/MediaUploadSetting usage) so TypeScript and runtime behavior are consistent.
168-175:⚠️ Potential issue | 🟠 MajorRemove unsafe type casting and bind the change handler to the input element instead of the form.
The
onFileChangehandler expects a file input change event but is currently cast unsafely from a form handler. While the handler accessese.target.files(which works at runtime), this type mismatch and unsafe casting (as unknown as) mask the intent and reduce type safety. Move theonChangehandler from the<form>to the<input>element to match the expected event type.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/koenig-lexical/src/components/ui/cards/VideoCard.tsx` around lines 168 - 175, The onFileChange handler is being unsafely cast on the <form> (as unknown as React.FormEventHandler) which masks the real input change type; move the onChange binding from the form to the file <input> element and remove the unsafe cast, i.e. attach onChange={onFileChange} to the input that uses ref={fileInputRef} and accept={videoMimeTypes.join(',')}, ensuring the handler receives the proper React.ChangeEvent<HTMLInputElement> rather than a FormEvent.packages/koenig-lexical/src/components/ui/cards/FileCard.tsx (1)
146-180:⚠️ Potential issue | 🟠 MajorThe empty-card branch should not accept missing upload props.
When
isPopulatedis false, the empty state unconditionally usesfileInputRefandonFileChangewith type assertions/non-null assertions (lines 147, 180) even though both are optional inFileCardProps.openFileSelection()will receive undefined iffileInputRefis omitted, andFileUploadFormwill fail at runtime ifonFileChangeis undefined since the component requires both props to be non-null.Model these as separate prop variants using a discriminated union type (where empty state requires both props), or enforce these props as required when
isPopulatedis false.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/koenig-lexical/src/components/ui/cards/FileCard.tsx` around lines 146 - 180, The empty-state branch of FileCard uses fileInputRef and onFileChange even though FileCardProps marks them optional; update FileCardProps to be a discriminated union (e.g. PopulatedProps vs EmptyProps) where when isPopulated is false the props fileInputRef and onFileChange are required, or add a runtime guard at the top of FileCard that throws/returns early if !isPopulated and either fileInputRef or onFileChange is missing; adjust handleOpenFileSelection (which calls openFileSelection) and the FileUploadForm invocation to rely on the now-required props (remove non-null assertions) so openFileSelection(fileInputRef) and <FileUploadForm fileInputRef={...} onFileChange={...}> are always safe when rendering the empty state.packages/koenig-lexical/src/components/ui/cards/GalleryCard.tsx (1)
240-247:⚠️ Potential issue | 🟠 MajorMove the
onChangehandler from<form>to<input>.The
onFileChangehandler is typed forReact.ChangeEvent<HTMLInputElement>and accessese.target.files, but placing it on the<form>element causese.targetto be anHTMLFormElement(which lacks afilesproperty). The castas unknown as React.FormEventHandler<HTMLFormElement>masks this mismatch. Attach the handler directly to the<input>instead.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/koenig-lexical/src/components/ui/cards/GalleryCard.tsx` around lines 240 - 247, The onFileChange handler is currently attached to the <form> (cast to React.FormEventHandler<HTMLFormElement>), which causes e.target to be an HTMLFormElement and breaks access to e.target.files; move the onChange handler to the <input> element (the input that uses fileInputRef and accepts imageMimeTypes) and remove the incorrect cast on the form so the handler remains typed as React.ChangeEvent<HTMLInputElement> and can safely read e.target.files.
♻️ Duplicate comments (10)
packages/koenig-lexical/src/nodes/header/v1/HeaderNodeComponent.tsx (1)
64-75:⚠️ Potential issue | 🟠 MajorGuard canceled file selection before mutating header image state.
HTMLInputElement.files(Line 65) is nullable. Right now, Line 71 clearsbackgroundImageSrcbefore validation, and Line 74 usesArray.from(files!). If picker is canceled, this can throw and also wipe the existing image unintentionally.Proposed fix
const onFileChange = async (e: React.ChangeEvent<HTMLInputElement>) => { const files = e.target.files; + if (!files || files.length === 0) { + return; + } // reset original src so it can be replaced with preview and upload progress editor.update(() => { const node = $getHeaderNodeByKey(nodeKey); if (!node) {return;} node.backgroundImageSrc = ''; }); - const result = await backgroundImageUploadHandler(Array.from(files!), imageUploader.upload); + const result = await backgroundImageUploadHandler(Array.from(files), imageUploader.upload); if (!result) {return;}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/koenig-lexical/src/nodes/header/v1/HeaderNodeComponent.tsx` around lines 64 - 75, In onFileChange, guard against a cancelled picker by checking e.target.files (files) is non-null and has length before calling editor.update or Array.from; move the null/length check to the top of onFileChange and early-return if files is falsy or empty, so backgroundImageUploadHandler(Array.from(files), imageUploader.upload) and the editor.update that clears node.backgroundImageSrc (via $getHeaderNodeByKey and nodeKey) only run when files exist and are valid.packages/koenig-lexical/src/nodes/VideoNodeComponent.tsx (1)
146-159:⚠️ Potential issue | 🟠 MajorGuard
imageUrlbefore reading thumbnail dimensions.
getImageDimensions(imageUrl!)still runs before the upload result is validated, so a failed/empty upload can throw before the handler exits.🛡️ Proposed fix
const handleCustomThumbnailChange = async (files: File[] | FileList) => { - const customThumbnailUploadResult = await customThumbnailUploader.upload(Array.from(files)); + const [file] = Array.from(files); + if (!file) {return;} + + const customThumbnailUploadResult = await customThumbnailUploader.upload([file]); const imageUrl = customThumbnailUploadResult?.[0]?.url; - const {width, height} = await getImageDimensions(imageUrl!); - - if (imageUrl) { - editor.update(() => { - const node = $getVideoNodeByKey(nodeKey); - if (!node) {return;} - node.customThumbnailSrc = imageUrl; - node.thumbnailWidth = width; - node.thumbnailHeight = height; - }); - } + if (!imageUrl) {return;} + + const {width, height} = await getImageDimensions(imageUrl); + editor.update(() => { + const node = $getVideoNodeByKey(nodeKey); + if (!node) {return;} + node.customThumbnailSrc = imageUrl; + node.thumbnailWidth = width; + node.thumbnailHeight = height; + }); };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/koenig-lexical/src/nodes/VideoNodeComponent.tsx` around lines 146 - 159, The handler handleCustomThumbnailChange calls getImageDimensions(imageUrl!) before verifying the upload result, which can throw if imageUrl is missing; change the flow to first await customThumbnailUploader.upload(Array.from(files)), then check customThumbnailUploadResult?.[0]?.url into imageUrl and return early if falsy, and only then call getImageDimensions(imageUrl) and run editor.update to set node.customThumbnailSrc, node.thumbnailWidth and node.thumbnailHeight (use $getVideoNodeByKey to fetch the node inside editor.update); this ensures getImageDimensions is never invoked with an undefined URL.packages/koenig-lexical/src/components/ui/TenorSelector.tsx (2)
132-138:⚠️ Potential issue | 🟡 MinorReturn after reaching the first GIF.
When
highlightedGif.index === 0, this still falls through togifs[-1]. If the intent is to move focus back to the search field, return afterfocusSearch(); if the intent is to clear the highlight, do that explicitly instead of relying on an invalid index.Suggested fix
function highlightPrev() { if (highlightedGif!.index === 0) { // reached the beginning, focus the search bar focusSearch(); + return; } setHighlightedGif(gifs[highlightedGif!.index - 1]); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/koenig-lexical/src/components/ui/TenorSelector.tsx` around lines 132 - 138, In highlightPrev, calling setHighlightedGif(gifs[highlightedGif!.index - 1]) runs even when highlightedGif!.index === 0 and tries to index -1; update highlightPrev (the function using highlightedGif, gifs, setHighlightedGif, and focusSearch) to either return immediately after focusSearch() so you don't call setHighlightedGif with an invalid index, or explicitly clear the highlight by calling setHighlightedGif(null) (or the intended "no selection" value) instead of relying on an invalid array access.
33-33:⚠️ Potential issue | 🟠 MajorRemap or clear
highlightedGifwhencolumnsreflow.
highlightedGifstorescolumnIndex/columnRowIndex, so a resize or fresh search result can leave it pointing at a column that no longer exists.moveHighlightUp()/moveHighlightDown()then dereferencecolumns[highlightedGif.columnIndex]before thenextGifguard runs.Also applies to: 142-150
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/koenig-lexical/src/components/ui/TenorSelector.tsx` at line 33, The highlightedGif state can point to invalid column indices after a columns reflow (resize or new search), causing moveHighlightUp/moveHighlightDown to dereference columns[highlightedGif.columnIndex]; add a safeguard that remaps or clears highlightedGif whenever columns change: in the TenorSelector component add an effect that checks highlightedGif against the current columns (and their lengths) and either clamps columnIndex/columnRowIndex to valid bounds or sets highlightedGif to undefined via setHighlightedGif; also update moveHighlightUp/moveHighlightDown to early-return if highlightedGif is out-of-range before accessing columns to fully prevent the dereference error.packages/koenig-lexical/src/components/ui/cards/CallToActionCard.tsx (1)
96-99:⚠️ Potential issue | 🟠 MajorOptional editor props still conflict with unconditional usage.
htmlEditorandsponsorLabelHtmlEditorare optional but both are force-unwrapped when rendering nested editors.Proposed fix
interface CallToActionCardProps { @@ - htmlEditor?: LexicalEditor; + htmlEditor: LexicalEditor; @@ - sponsorLabelHtmlEditor?: LexicalEditor; + sponsorLabelHtmlEditor: LexicalEditor; @@ - initialEditor={sponsorLabelHtmlEditor!} + initialEditor={sponsorLabelHtmlEditor} @@ - initialEditor={htmlEditor!} + initialEditor={htmlEditor}Also applies to: 374-375, 421-422
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/koenig-lexical/src/components/ui/cards/CallToActionCard.tsx` around lines 96 - 99, The CallToActionCard component declares htmlEditor and sponsorLabelHtmlEditor as optional but uses them unconditionally when rendering nested editors; update the render logic in CallToActionCard to null-check these props (htmlEditor and sponsorLabelHtmlEditor) before invoking or passing them to nested editor components (or render a fallback/no-op editor when absent) and remove any non-null assertions; ensure the same pattern is applied where those props are referenced (including the nested editor render blocks around the previous usages at the bottom of the component and the sponsor label block) so the component safely handles undefined editors.packages/koenig-lexical/demo/DemoApp.tsx (2)
297-302:⚠️ Potential issue | 🟠 MajorStop reaching into Lexical internals for the root element.
Line 297 is still asserting
_rootElementand Line 301 reads it directly.LexicalEditorexposesgetRootElement()as the public API, while_rootElementis an internal field, so this cast is brittle across Lexical upgrades. (github.com)Suggested fix
- const editor = editorAPI.editorInstance as {_rootElement: HTMLElement; getEditorState: () => {read: (fn: () => void) => void}}; + const editor = editorAPI.editorInstance as { + getRootElement?: () => HTMLElement | null; + getEditorState: () => {read: (fn: () => void) => void}; + }; + const rootElement = editor.getRootElement?.(); + if (!rootElement) { + skipFocusEditor.current = false; + return; + } // if a mousedown and subsequent mouseup occurs below the editor // canvas, focus the editor and put the cursor at the end of the document - const {bottom} = editor._rootElement.getBoundingClientRect(); + const {bottom} = rootElement.getBoundingClientRect();For the current official Lexical API, is `_rootElement` an internal field and is `getRootElement()` the supported public way to access the editor root element?🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/koenig-lexical/demo/DemoApp.tsx` around lines 297 - 302, The code is reaching into Lexical internals by casting editorAPI.editorInstance and reading _rootElement directly; replace uses of _rootElement with the public API getRootElement() (call editorAPI.editorInstance.getRootElement()) to obtain the root HTMLElement, then use its getBoundingClientRect() to compute bottom and perform the event.pageY / event.clientY checks as before; update any type assertions around editorInstance to use the LexicalEditor type or a safer optional check for getRootElement() returning an HTMLElement before using it (refer to editorAPI.editorInstance, _rootElement, and getRootElement).
343-347:⚠️ Potential issue | 🟠 MajorGuard
saveContent()until the editor API is ready.Line 344 can still throw if the sidebar action fires before
registerAPIcompletes, and a failedserialize()currently propagates out of the click handler.Suggested fix
function saveContent() { - const serializedState = editorAPI!.serialize(); + if (!editorAPI) { + return; + } + + let serializedState: string; + try { + serializedState = editorAPI.serialize(); + } catch { + return; + } + const encodedContent = encodeURIComponent(serializedState); searchParams.set('content', encodedContent); setSearchParams(searchParams); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/koenig-lexical/demo/DemoApp.tsx` around lines 343 - 347, The saveContent function can throw if editorAPI isn't ready; update saveContent to guard against a missing or not-yet-registered editorAPI (the value set by registerAPI) before calling editorAPI.serialize(), and wrap the serialize() and encodeURIComponent() steps in a try/catch so serialization failures don't propagate out of the click handler; if editorAPI is absent, no-op or defer, and on error log/handle it gracefully before calling setSearchParams with the encoded content.packages/koenig-lexical/src/nodes/CallToActionNodeComponent.tsx (1)
144-145:⚠️ Potential issue | 🔴 CriticalHandle cancelled or empty file input before reading
files[0].Lines 145 and 163 still assume a file exists. Cancelling the picker or receiving an empty file payload will dereference
undefinedand throw.Suggested fix
const handleImageChange = async (files: FileList | File[]) => { + if (files.length === 0) { + return; + } const imgPreviewUrl = URL.createObjectURL(files[0]); try { const {width, height} = await getImageDimensions(imgPreviewUrl); const result = await imageUploader.upload(Array.from(files)); @@ const onFileChange = async (e: React.ChangeEvent<HTMLInputElement>) => { - handleImageChange(e.target.files!); + const files = e.target.files; + if (!files || files.length === 0) { + return; + } + await handleImageChange(files); };Also applies to: 162-163
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/koenig-lexical/src/nodes/CallToActionNodeComponent.tsx` around lines 144 - 145, The handler handleImageChange currently assumes files[0] exists and will throw if the picker was cancelled or an empty list is passed; add a guard at the start of handleImageChange that returns early (or handles error) when files is falsy or files.length === 0, and only then create the preview URL and proceed with reading/uploading; apply the same guard where files[0] is referenced elsewhere in this component to avoid dereferencing undefined.packages/koenig-lexical/src/components/ui/SettingsPanel.tsx (1)
329-345:⚠️ Potential issue | 🟠 Major
showChildrenis still a dead prop here.
ColorPickerSettingPropsexposesshowChildren, but the component never reads it and always renderschildren. That makes the new prop misleading and prevents callers from hiding the slot.Suggested fix
-export function ColorPickerSetting({label, isExpanded, onSwatchChange, onPickerChange, onTogglePicker, value, swatches, eyedropper, hasTransparentOption, dataTestId, children}: ColorPickerSettingProps) { +export function ColorPickerSetting({label, isExpanded, onSwatchChange, onPickerChange, onTogglePicker, value, swatches, eyedropper, hasTransparentOption, dataTestId, children, showChildren = true}: ColorPickerSettingProps) { @@ > - {children} + {showChildren ? children : null} </ColorIndicator>Also applies to: 364-365
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/koenig-lexical/src/components/ui/SettingsPanel.tsx` around lines 329 - 345, The prop showChildren on ColorPickerSettingProps is unused while the ColorPickerSetting component always renders children; either remove the prop or use it to control rendering — update the ColorPickerSetting function to accept showChildren and wrap the children render with a conditional (e.g., if (showChildren) render children) so callers can hide the slot, ensuring you reference the showChildren prop alongside existing props like children, onSwatchChange, onPickerChange and onTogglePicker when making the change.packages/koenig-lexical/src/components/ui/cards/AudioCard.tsx (1)
42-48:⚠️ Potential issue | 🟠 MajorOptional
onFileChangeprops still used with non-null assertions.The interfaces declare
onFileChangeas optional (?), but lines 84 and 177 use non-null assertions (onFileChange!) when passing toAudioUploadFormandImageUploadForm. If the handler is omitted, this will crash at runtime.The previous review suggested making these handlers required. Consider either:
- Making
onFileChangerequired in the interfaces, or- Adding runtime guards before invoking
Also applies to: 91-102, 184-197
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/koenig-lexical/src/components/ui/cards/AudioCard.tsx` around lines 42 - 48, The optional prop onFileChange in EmptyAudioCardProps is being force-unwrapped when passed to AudioUploadForm and ImageUploadForm, risking runtime crashes; remove the non-null assertions and either (a) make onFileChange required in the interface (EmptyAudioCardProps) and update all callers, or (b) add runtime guards/defaults where you pass it (in the component JSX that renders AudioUploadForm and ImageUploadForm) by passing a safe handler: e.g., pass onFileChange only if defined or pass a noop wrapper like (e) => onFileChange?.(e) so the forms always receive a callable handler without changing the prop contract; update both occurrences where onFileChange is currently asserted.
🟡 Minor comments (13)
packages/koenig-lexical/src/components/ui/cards/TransistorCard.tsx-29-29 (1)
29-29:⚠️ Potential issue | 🟡 MinorLine 29 uses an invalid line-height utility.
leading-[-0.08px]is not valid CSS (negative line-height is invalid), so this rule will be ignored. Usetracking-[-0.08px]instead if the intent is letter spacing adjustment.Suggested correction
- <div className="text-center text-[2.1rem] font-semibold leading-[-0.08px] text-black dark:text-white"> + <div className="text-center text-[2.1rem] font-semibold tracking-[-0.08px] text-black dark:text-white">🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/koenig-lexical/src/components/ui/cards/TransistorCard.tsx` at line 29, The div in TransistorCard.tsx uses an invalid CSS utility leading-[-0.08px] (negative line-height is invalid and will be ignored); update the className on the element in TransistorCard (the text-center text-[2.1rem] font-semibold ... div) to replace leading-[-0.08px] with tracking-[-0.08px] (or remove it if letter-spacing isn't needed) so the intended letter-spacing takes effect.packages/koenig-lexical/src/nodes/VideoNode.tsx-47-57 (1)
47-57:⚠️ Potential issue | 🟡 MinorValidate
initialFilebefore storing it on the node.
datasetnow accepts arbitrary values, but this assignment treats any truthyinitialFileas aFile. A bad payload will reachhandleVideoUpload, which immediately readsfile.nameand passes it into metadata extraction/upload code.🔎 Proposed fix
constructor(dataset: Record<string, unknown> = {}, key?: string) { super(dataset, key); const {triggerFileDialog, initialFile} = dataset; @@ - this.__initialFile = (initialFile as File | null) || null; + this.__initialFile = initialFile instanceof File ? initialFile : null;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/koenig-lexical/src/nodes/VideoNode.tsx` around lines 47 - 57, In the VideoNode constructor, validate dataset.initialFile before storing it: only assign to this.__initialFile if dataset.initialFile is an actual File (e.g., using instanceof File or another robust type check); otherwise set this.__initialFile = null. Update any related assumptions (e.g., handleVideoUpload and places that access __initialFile or file.name) so they guard against null and never assume a truthy value is a File.packages/koenig-lexical/src/nodes/VideoNodeComponent.tsx-74-91 (1)
74-91:⚠️ Potential issue | 🟡 MinorClear stale metadata errors before retrying an upload.
metadataExtractionErrorsis only set in the failure path here, so a later successful upload keeps rendering the previous error viavideoUploadErrors. Reset it before re-processing the next file.🧹 Proposed fix
const handleVideoUpload = async (files: File[] | FileList) => { const file = files[0]; if (!file) { return; } + setMetadataExtractionErrors([]); let thumbnailBlob: Blob | null | undefined; let duration: number | undefined; let width: number | undefined; let height: number | undefined; let mimeType: string | undefined;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/koenig-lexical/src/nodes/VideoNodeComponent.tsx` around lines 74 - 91, The metadataExtractionErrors state isn't cleared when a new upload starts, so previous errors persist after a successful retry; inside handleVideoUpload (before extracting metadata and/or before the try block) call setMetadataExtractionErrors to reset/clear the errors (e.g. setMetadataExtractionErrors([]) or equivalent) so stale errors are removed before calling extractVideoMetadata(file); ensure you reference the existing handleVideoUpload and setMetadataExtractionErrors symbols when making the change.packages/koenig-lexical/src/components/ui/cards/HeaderCard/v1/HeaderCard.tsx-245-246 (1)
245-246:⚠️ Potential issue | 🟡 MinorNon-null assertion on optional prop may cause runtime error.
handleButtonUrlis declared as optional inHeaderCardV1Props(line 39), but line 246 uses a non-null assertion (handleButtonUrl!). If the parent doesn't provide this callback, this will throw at runtime when the user changes the button URL.Consider either making
handleButtonUrlrequired whenbuttonis true, or guarding the call:🛡️ Proposed safer approach
<InputUrlSetting dataTestId='header-button-url' label='Button URL' value={buttonUrl || ''} - onChange={handleButtonUrl!} + onChange={handleButtonUrl ?? (() => {})} />Or update the interface to make it required when button functionality is used.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/koenig-lexical/src/components/ui/cards/HeaderCard/v1/HeaderCard.tsx` around lines 245 - 246, The component currently uses a non-null assertion on the optional prop handleButtonUrl in HeaderCardV1 (onChange={handleButtonUrl!}), which can throw if the parent omits the callback; fix by either (A) guarding the onChange so it only calls/assigns when handleButtonUrl exists (e.g., conditionally pass onChange or wrap the handler), or (B) change the props type HeaderCardV1Props to require handleButtonUrl when button is true (use a discriminated union or make handleButtonUrl mandatory in the “with button” variant) and update usages accordingly so the compiler enforces the presence of handleButtonUrl when the button is rendered.packages/koenig-lexical/demo/RestrictedContentDemo.tsx-89-89 (1)
89-89:⚠️ Potential issue | 🟡 MinorNon-null assertions could cause runtime errors.
Using
containerRef.current!assumes the ref is always attached when this code runs. IffocusEditoris called before the div mounts or after unmount, this will throw.Consider using optional chaining with an early return:
🛡️ Proposed safer access
- containerRef.current!.scrollTop = containerRef.current!.scrollHeight; + if (containerRef.current) { + containerRef.current.scrollTop = containerRef.current.scrollHeight; + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/koenig-lexical/demo/RestrictedContentDemo.tsx` at line 89, The code uses a non-null assertion on containerRef.current when setting scrollTop inside focusEditor which can throw if the div isn't mounted; update focusEditor to guard access by returning early if containerRef.current is falsy (e.g., if (!containerRef.current) return) or use optional chaining before setting scrollTop, ensuring you reference the containerRef symbol and the focusEditor function so the scrollTop assignment is only performed when the element exists.packages/koenig-lexical/src/components/KoenigComposer.tsx-149-152 (1)
149-152:⚠️ Potential issue | 🟡 MinorType mismatch:
initialEditorStatecan be object but cast to string.
initialEditorStateis typed asstring | Record<string, unknown>in props (line 30), but line 151 casts it asstring | undefinedforCollaborationPlugin. If an object is passed, this could cause unexpected behavior.Consider serializing it consistently:
🛡️ Proposed fix
<CollaborationPlugin id="main" - initialEditorState={initialEditorState as string | undefined} + initialEditorState={typeof initialEditorState === 'string' ? initialEditorState : initialEditorState ? JSON.stringify(initialEditorState) : undefined} providerFactory={createWebsocketProvider as unknown as Parameters<typeof CollaborationPlugin>[0]['providerFactory']}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/koenig-lexical/src/components/KoenigComposer.tsx` around lines 149 - 152, initialEditorState prop on KoenigComposer is typed as string | Record<string, unknown> but is being force-cast to string | undefined for CollaborationPlugin, which will break when an object is passed; instead, ensure you pass a string or undefined by serializing objects (e.g. JSON.stringify) before handing them to CollaborationPlugin's initialEditorState prop (or explicitly map non-string values to undefined), and update the usage in the component where initialEditorState is passed to CollaborationPlugin (look for the CollaborationPlugin component and the initialEditorState prop) so the value is always a string or undefined without unsafe casting.packages/koenig-lexical/src/nodes/BookmarkNodeComponent.tsx-121-123 (1)
121-123:⚠️ Potential issue | 🟡 MinorDuplicate null check - likely copy-paste error.
Lines 122 and 123 both check
if (!node) {return;}. The second check is redundant.🐛 Proposed fix
editor.update(() => { const node = $getBookmarkNodeByKey(nodeKey); - if (!node) {return;} if (!node) {return;} node.url = href;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/koenig-lexical/src/nodes/BookmarkNodeComponent.tsx` around lines 121 - 123, The code in BookmarkNodeComponent.tsx calls $getBookmarkNodeByKey(nodeKey) and then performs the same null-check twice; remove the redundant `if (!node) { return; }` so there is only a single null guard after the $getBookmarkNodeByKey(nodeKey) call (leave the first check in place and delete the duplicate) to avoid dead/repeated checks while preserving subsequent logic that relies on `node`.packages/koenig-lexical/src/nodes/BookmarkNodeComponent.tsx-147-149 (1)
147-149:⚠️ Potential issue | 🟡 MinorDuplicate null check here as well.
Same issue as above - lines 148 and 149 have duplicate
if (!node) {return;}checks.🐛 Proposed fix
editor.update(() => { const node = $getBookmarkNodeByKey(nodeKey); - if (!node) {return;} if (!node) {return;} node.url = response.url;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/koenig-lexical/src/nodes/BookmarkNodeComponent.tsx` around lines 147 - 149, There are two identical null checks for the bookmark node after calling $getBookmarkNodeByKey(nodeKey); remove the redundant check so only a single `if (!node) { return; }` remains; update the block around the `node` variable (the code that calls $getBookmarkNodeByKey and checks `node`) to keep one guard and delete the duplicate to avoid unreachable/duplicated logic in BookmarkNodeComponent.packages/koenig-lexical/demo/RestrictedContentDemo.tsx-59-60 (1)
59-60:⚠️ Potential issue | 🟡 MinorUse the public
getRootElement()API instead of accessing internal_rootElementproperty.The
_rootElementproperty is internal to Lexical (indicated by the underscore prefix). Throughout the codebase,getRootElement()is the standard public API for accessing the root element—use it here as well. Update line 60 to:const {bottom} = editor.getRootElement()?.getBoundingClientRect() ?? {bottom: 0};Or handle the null case appropriately for your use case.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/koenig-lexical/demo/RestrictedContentDemo.tsx` around lines 59 - 60, The code accesses the internal property editorAPI.editorInstance._rootElement; replace this with the public API by calling editorAPI.editorInstance.getRootElement() and use its getBoundingClientRect() safely (handle possible null) instead of touching _rootElement; update the code around editorAPI.editorInstance and _rootElement to call getRootElement() and fall back (e.g., {bottom:0} or appropriate handling) when getRootElement() returns null.packages/koenig-lexical/src/components/ui/LinkActionToolbar.tsx-19-24 (1)
19-24:⚠️ Potential issue | 🟡 Minor
setTextNodeRangerequires aTextNodeargument; add a type guard.
selection.focus.getNode()returns anyLexicalNode, not just aTextNode. The code should check the node type before callingsetTextNodeRange. This issue exists in bothLinkActionToolbar.tsxandLinkActionToolbarWithSearch.tsxand should be fixed in both locations.🛡️ Proposed fix
+import {$createRangeSelection, $getSelection, $isRangeSelection, $isTextNode, $setSelection} from 'lexical'; ... if (selection && $isRangeSelection(selection)) { const focusNode = selection.focus.getNode(); - const rangeSelection = $createRangeSelection(); - rangeSelection.setTextNodeRange(focusNode, focusNode.getTextContentSize(), focusNode, focusNode.getTextContentSize()); - $setSelection(rangeSelection); + if ($isTextNode(focusNode)) { + const rangeSelection = $createRangeSelection(); + rangeSelection.setTextNodeRange(focusNode, focusNode.getTextContentSize(), focusNode, focusNode.getTextContentSize()); + $setSelection(rangeSelection); + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/koenig-lexical/src/components/ui/LinkActionToolbar.tsx` around lines 19 - 24, selection.focus.getNode() may return any LexicalNode but setTextNodeRange expects a TextNode, so add a type guard before calling rangeSelection.setTextNodeRange: after obtaining selection (via $getSelection() and verifying $isRangeSelection(selection)), get the node with selection.focus.getNode() and check it is a TextNode (use the lexical helper like $isTextNode or node.getType() === 'text') then call $createRangeSelection(), rangeSelection.setTextNodeRange(...) and $setSelection(rangeSelection) only when the node passes the check; apply this same guard in both LinkActionToolbar.tsx and LinkActionToolbarWithSearch.tsx to avoid calling setTextNodeRange on non-text nodes.packages/koenig-lexical/src/components/ui/ColorPicker.tsx-63-66 (1)
63-66:⚠️ Potential issue | 🟡 MinorHandle user eyedropper cancellation to avoid unhandled promise rejection.
The EyeDropper Web API rejects with an
AbortErrorDOMException when the user cancels the picker (e.g., by pressing Esc). Without a.catch()block, this rejection remains unhandled despite.finally()still cleaning up the UI state. Add error handling to silently ignore expectedAbortErrorcancellations while logging unexpected failures.Suggested fix
const eyeDropper = new (window as unknown as {EyeDropper: new () => {open: () => Promise<{sRGBHex: string}>}}).EyeDropper(); eyeDropper.open() .then((result: {sRGBHex: string}) => onChange(result.sRGBHex)) + .catch((error: unknown) => { + if (!(error instanceof DOMException && error.name === 'AbortError')) { + console.error(error); + } + }) .finally(() => { isUsingColorPicker.current = false; document.body.style.removeProperty('pointer-events');🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/koenig-lexical/src/components/ui/ColorPicker.tsx` around lines 63 - 66, The EyeDropper.open() promise can reject with an AbortError when the user cancels; update the chain on the EyeDropper instance (eyeDropper.open()) to add a .catch handler that silently ignores DOMExceptions with error.name === 'AbortError' (or equivalent) and forwards/logs any other unexpected errors (e.g., using console.error or the component logger), while preserving the existing .finally() cleanup; reference the eyeDropper variable and the onChange callback so the success path still calls onChange(result.sRGBHex).packages/koenig-lexical/src/components/ui/cards/GalleryCard.tsx-17-25 (1)
17-25:⚠️ Potential issue | 🟡 MinorUse
image.fileNameas the key instead ofimage.src.Using
image.srcis unsafe: it's undefined during the upload phase (images initially have onlypreviewSrc), and if the same file is added twice, React will create duplicate keys and can reuse the wrong element during delete/reorder operations. Since the delete logic already usesfileNamefor filtering, key each tile with that property for consistency and stability.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/koenig-lexical/src/components/ui/cards/GalleryCard.tsx` around lines 17 - 25, In GalleryRow, the GalleryImageComponent is keyed with image.src which can be undefined during upload and causes duplicate keys if the same file is added twice; change the key to use image.fileName (the same identifier used by the delete logic) by replacing key={image.src} with key={image.fileName} on the GalleryImageComponent to ensure stable, unique keys during upload, delete and reorder operations.packages/koenig-lexical/src/nodes/FileNodeComponent.tsx-74-74 (1)
74-74:⚠️ Potential issue | 🟡 MinorGuard against null
filesbefore using non-null assertion.
e.target.filescan benullif no file was selected. The non-null assertionfiles!could cause a runtime error.Proposed fix
const onFileChange = async (e: React.ChangeEvent<HTMLInputElement>) => { const files = e.target.files; + if (!files || files.length === 0) { + return; + } // reset original src so it can be replaced with preview and upload progress editor.update(() => { const node = $getNodeByKey(nodeKey) as FileNode | null; if (!node) {return;} node.src = ''; }); - return await fileUploadHandler(Array.from(files!), nodeKey, editor, uploader.upload); + return await fileUploadHandler(Array.from(files), nodeKey, editor, uploader.upload); };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/koenig-lexical/src/nodes/FileNodeComponent.tsx` at line 74, Guard against e.target.files being null before using the non-null assertion; in the upload flow where FileNodeComponent calls fileUploadHandler, check e.target.files (or the local variable files) for null/undefined and return or handle the empty case instead of calling fileUploadHandler(Array.from(files!), nodeKey, editor, uploader.upload). Update the branch that references files to only call fileUploadHandler when files is non-null and has entries (use files.length or a null check) and pass nodeKey, editor, and uploader.upload as before.
| files: ['**/*.{ts,tsx}'], | ||
| extends: [ | ||
| js.configs.recommended, | ||
| tseslint.configs.recommended, | ||
| reactHooks.configs['recommended-latest'] | ||
| ], |
There was a problem hiding this comment.
Don’t drop lint coverage for the remaining JSX modules.
This config now matches only ts/tsx, but the package still imports .jsx source files such as MarkdownPastePlugin.jsx and TKPlugin.jsx. Those files will stop being linted entirely during the migration unless js/jsx stay included.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/koenig-lexical/eslint.config.js` around lines 11 - 16, The files
pattern in eslint.config.js currently restricts linting to '**/*.{ts,tsx}',
which will omit existing .jsx sources like MarkdownPastePlugin.jsx and
TKPlugin.jsx; update the files glob used in the config (the files array entry)
to include JavaScript extensions (e.g., '**/*.{js,jsx,ts,tsx}' or add a separate
'**/*.{js,jsx}' entry) so those modules remain linted by the existing ruleset.
| const provider = new WebsocketProvider( | ||
| multiplayerEndpoint, | ||
| multiplayerDocId + '/' + id, | ||
| multiplayerEndpoint!, | ||
| (multiplayerDocId || '') + '/' + id, |
There was a problem hiding this comment.
Non-null assertion on multiplayerEndpoint may cause runtime error.
When enableMultiplayer is true (line 148), createWebsocketProvider is used, but multiplayerEndpoint is optional in the props interface. The ! assertion on line 115 will throw if the endpoint isn't provided.
Consider adding a runtime check or making multiplayerEndpoint required when enableMultiplayer is true:
🛡️ Proposed runtime guard
+ if (enableMultiplayer && !multiplayerEndpoint) {
+ console.error('<KoenigComposer> requires `multiplayerEndpoint` when `enableMultiplayer` is true');
+ }
+
const createWebsocketProvider = React.useCallback((id: string, yjsDocMap: Map<string, Doc>) => {
+ if (!multiplayerEndpoint) {
+ throw new Error('multiplayerEndpoint is required for multiplayer mode');
+ }
let doc = yjsDocMap.get(id);
...
const provider = new WebsocketProvider(
- multiplayerEndpoint!,
+ multiplayerEndpoint,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/koenig-lexical/src/components/KoenigComposer.tsx` around lines 114 -
116, The code uses a non-null assertion on multiplayerEndpoint when creating the
WebsocketProvider (new WebsocketProvider(...)) which can crash at runtime if the
prop is omitted; update createWebsocketProvider / the KoenigComposer component
to validate multiplayerEndpoint before using it (e.g., if enableMultiplayer is
true, throw a clear error or early-return and log when multiplayerEndpoint is
missing) or make multiplayerEndpoint required in that usage path; ensure the
check covers the call sites that construct WebsocketProvider and references to
multiplayerDocId/id so WebsocketProvider is only constructed when
multiplayerEndpoint is present.
| interface CalloutCardProps { | ||
| color?: CalloutColor; | ||
| isEditing?: boolean; | ||
| setShowEmojiPicker: (show: boolean) => void; | ||
| toggleEmoji?: (event: React.ChangeEvent<HTMLInputElement>) => void; | ||
| hasEmoji?: boolean; | ||
| handleColorChange?: (name: string) => void; | ||
| changeEmoji?: (emoji: {native: string}) => void; | ||
| calloutEmoji?: string; | ||
| textEditor?: LexicalEditor; | ||
| textEditorInitialState?: string; | ||
| nodeKey?: string; | ||
| toggleEmojiPicker?: () => void; | ||
| showEmojiPicker?: boolean; | ||
| } |
There was a problem hiding this comment.
Make textEditor and handleColorChange required in CalloutCardProps.
These properties are used without null checks (with ! assertions) and are always provided by the parent component. Marking them optional while enforcing their presence at runtime defeats type safety. Remove the ? from lines 92 and 95, and remove the corresponding ! assertions at lines 155 and 180.
Proposed fix
interface CalloutCardProps {
color?: CalloutColor;
isEditing?: boolean;
setShowEmojiPicker: (show: boolean) => void;
toggleEmoji?: (event: React.ChangeEvent<HTMLInputElement>) => void;
hasEmoji?: boolean;
- handleColorChange?: (name: string) => void;
+ handleColorChange: (name: string) => void;
changeEmoji?: (emoji: {native: string}) => void;
calloutEmoji?: string;
- textEditor?: LexicalEditor;
+ textEditor: LexicalEditor;
textEditorInitialState?: string;
nodeKey?: string;
toggleEmojiPicker?: () => void;
showEmojiPicker?: boolean;
}Also remove ! at lines 155 and 180.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/koenig-lexical/src/components/ui/cards/CalloutCard.tsx` around lines
87 - 101, Update the CalloutCardProps interface to make textEditor and
handleColorChange required (remove the ? from textEditor and handleColorChange)
and then remove the corresponding non-null assertions (!) where these props are
used (remove the "!" usages on textEditor and handleColorChange in the
CalloutCard component so the code relies on the now-required props directly).
Ensure function signatures and any call sites inside CalloutCard reference
textEditor and handleColorChange as non-optional members of CalloutCardProps.
| // show the language input when the mouse moves | ||
| React.useEffect(() => { | ||
| const onMouseMove = () => { | ||
| setShowLanguage(true); | ||
| }; | ||
|
|
||
| window.addEventListener('mousemove', onMouseMove); | ||
|
|
||
| return () => { | ||
| window.removeEventListener('mousemove', onMouseMove); | ||
| }; | ||
| }, []); |
There was a problem hiding this comment.
Scope the language-toggle listener to this card.
Binding mousemove on window means any mouse movement anywhere in the app re-shows the language field. It also installs a high-frequency global listener per mounted editor. Please move this to the card container with onMouseEnter/onMouseMove instead.
Proposed fix
- // show the language input when the mouse moves
- React.useEffect(() => {
- const onMouseMove = () => {
- setShowLanguage(true);
- };
-
- window.addEventListener('mousemove', onMouseMove);
-
- return () => {
- window.removeEventListener('mousemove', onMouseMove);
- };
- }, []);
-
const onChange = React.useCallback((value: string) => {
setShowLanguage(false); // hide language input whenever the user types in the editor
updateCode?.(value);
@@
- <div className="not-kg-prose min-h-[170px]">
+ <div className="not-kg-prose min-h-[170px]" onMouseEnter={() => setShowLanguage(true)}>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // show the language input when the mouse moves | |
| React.useEffect(() => { | |
| const onMouseMove = () => { | |
| setShowLanguage(true); | |
| }; | |
| window.addEventListener('mousemove', onMouseMove); | |
| return () => { | |
| window.removeEventListener('mousemove', onMouseMove); | |
| }; | |
| }, []); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/koenig-lexical/src/components/ui/cards/CodeBlockCard.tsx` around
lines 27 - 38, The current useEffect installs a global window 'mousemove'
listener that calls setShowLanguage; replace this by scoping the hover handlers
to the card container: remove the window.addEventListener logic in the
useEffect, and instead attach onMouseEnter and onMouseMove handlers (or add a
ref and attach listeners to the container element) in the CodeBlockCard
component that call setShowLanguage(true); ensure any cleanup (if using a ref
and addEventListener) removes the handlers, and keep existing state updates
(setShowLanguage) and other logic unchanged.
| <div className="not-kg-prose min-h-[170px]"> | ||
| <CodeMirror | ||
| autoFocus={true} // autofocus the editor whenever it is rendered | ||
| basicSetup={false} // basic setup includes unnecessary extensions | ||
| extensions={extensions} | ||
| value={code} | ||
| onChange={onChange} | ||
| /> | ||
| <input | ||
| aria-label="Code card language" | ||
| className={`z-999 absolute right-1.5 top-1.5 w-1/5 rounded-md border border-grey-300 px-2 py-1 font-sans text-[1.3rem] leading-4 text-grey-900 transition-opacity focus-visible:outline-none dark:border-grey-900 dark:text-grey-400 ${showLanguage ? 'opacity-100' : 'opacity-0'}`} | ||
| data-testid="code-card-language" | ||
| placeholder="Language..." | ||
| type="text" | ||
| value={language} | ||
| onChange={onLanguageChange} | ||
| /> |
There was a problem hiding this comment.
Add a local positioning context for the overlays.
Both language overlays are absolutely positioned, but neither wrapper establishes a positioning context. Their placement currently depends on some ancestor outside this component, so they can drift as layout changes.
Proposed fix
- <div className="not-kg-prose min-h-[170px]">
+ <div className="relative not-kg-prose min-h-[170px]">
@@
- <div className="not-kg-prose">
+ <div className="relative not-kg-prose">Also applies to: 229-237
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/koenig-lexical/src/components/ui/cards/CodeBlockCard.tsx` around
lines 197 - 213, The absolutely positioned overlay elements (the language input
with data-testid "code-card-language" and the other overlay around lines
229-237) lack a local positioning context; update the wrapper(s) in the
CodeBlockCard component (the div currently using className "not-kg-prose
min-h-[170px]" and the sibling wrapper that contains the other overlay) to
establish a local stacking context by adding a relative positioning class or
style (e.g., add "relative" to the wrapper className or use style={{position:
'relative'}}) so the absolute-positioned overlays are positioned relative to the
CodeBlockCard instead of ancestor elements.
| const panelRepositionDebounced = debounce((newWidth: number) => { | ||
| prevWidth = newWidth; | ||
| onResize(ref.current); | ||
| onResize(ref.current!); | ||
| }, 100, {leading: true, trailing: true}); |
There was a problem hiding this comment.
Cancel the debounced resize callback on cleanup.
Disconnecting the ResizeObserver is not enough here. The trailing debounce can still fire after cleanup, dereference ref.current!, and call setPosition against an unmounted panel. Cancel the debounce in the cleanup and guard ref.current inside the callback.
Suggested fix
const panelRepositionDebounced = debounce((newWidth: number) => {
prevWidth = newWidth;
- onResize(ref.current!);
+ if (ref.current) {
+ onResize(ref.current);
+ }
}, 100, {leading: true, trailing: true});
@@
return () => {
resizeObserver.disconnect();
+ panelRepositionDebounced.cancel();
};Also applies to: 260-262
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/koenig-lexical/src/hooks/useSettingsPanelReposition.ts` around lines
242 - 245, The debounced resize callback (panelRepositionDebounced) can fire
after cleanup and dereference ref.current; modify the debounce callback(s) (the
one assigning prevWidth and calling onResize(ref.current!)) to first check that
ref.current is non-null before using it, and in the hook cleanup call
panelRepositionDebounced.cancel() (and any other debounced instance around lines
~260-262) so the trailing invocation is cancelled; keep references to the
functions/promise (panelRepositionDebounced) so you can call .cancel() and
ensure prevWidth/onResize logic only runs when ref.current is present.
| const node = $getNodeByKey(nodeKey) as ButtonNode | null; | ||
| if (!node) {return;} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify whether a ButtonNode runtime type guard exists and how node narrowing is done elsewhere.
rg -n --type=ts --type=tsx '\$isButtonNode\s*\('
rg -n --type=ts --type=tsx '\$getNodeByKey\([^)]*\)\s+as\s+ButtonNode'Repository: TryGhost/Koenig
Length of output: 114
🏁 Script executed:
# Read the ButtonNodeComponent.tsx file to see the actual code
cat -n packages/koenig-lexical/src/nodes/ButtonNodeComponent.tsx | head -60Repository: TryGhost/Koenig
Length of output: 2746
🏁 Script executed:
# Search for $isButtonNode without strict type filtering
rg -n '\$isButtonNode' packages/koenig-lexical/Repository: TryGhost/Koenig
Length of output: 177
🏁 Script executed:
# Search for how $getNodeByKey is used with type assertions
rg -n 'as ButtonNode' packages/koenig-lexical/Repository: TryGhost/Koenig
Length of output: 451
🏁 Script executed:
# Check what's exported from kg-default-nodes package related to ButtonNode
fd kg-default-nodes -type dRepository: TryGhost/Koenig
Length of output: 229
🏁 Script executed:
# Check ButtonNode.tsx to see how $isButtonNode is defined and exported
cat -n packages/koenig-lexical/src/nodes/ButtonNode.tsx | sed -n '40,60p'Repository: TryGhost/Koenig
Length of output: 412
🏁 Script executed:
# Check if there's a kg-default-nodes package in the repo and what it exports
find . -name "*kg-default-nodes*" -type d | head -5Repository: TryGhost/Koenig
Length of output: 86
🏁 Script executed:
# Look at other node component files to see the pattern they use
ls -la packages/koenig-lexical/src/nodes/*Component.tsx | head -10Repository: TryGhost/Koenig
Length of output: 1147
🏁 Script executed:
# Check one other node component for comparison on how it handles type guards
head -50 packages/koenig-lexical/src/nodes/ImageNodeComponent.tsxRepository: TryGhost/Koenig
Length of output: 2718
🏁 Script executed:
# Check what's in the kg-default-nodes package
ls -la packages/kg-default-nodes/Repository: TryGhost/Koenig
Length of output: 718
🏁 Script executed:
# Check if kg-default-nodes re-exports ButtonNode and $isButtonNode
grep -r "ButtonNode\|isButtonNode" packages/kg-default-nodes/ --include="*.ts" --include="*.tsx" --include="*.js" | head -20Repository: TryGhost/Koenig
Length of output: 2106
🏁 Script executed:
# Check how other node components import and use type guards
grep -n "\$is.*Node\|import.*\$is" packages/koenig-lexical/src/nodes/*Component.tsx | head -20Repository: TryGhost/Koenig
Length of output: 706
🏁 Script executed:
# Check ButtonNode.tsx full structure to understand exports
cat -n packages/koenig-lexical/src/nodes/ButtonNode.tsx | head -50Repository: TryGhost/Koenig
Length of output: 1882
Prefer runtime type guard over unchecked as ButtonNode assertions
At lines 33, 41, and 49, the type casts narrow at compile time only. Use the locally exported $isButtonNode type guard before mutation for runtime safety. However, the type guard should be imported from the local ./ButtonNode module, not from @tryghost/kg-default-nodes, since the nodes in the editor are instances of the local ButtonNode class (which extends the external base class). Import {$isButtonNode} from './ButtonNode' and replace the three as ButtonNode | null assertions with a check using $isButtonNode(node).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/koenig-lexical/src/nodes/ButtonNodeComponent.tsx` around lines 33 -
34, Replace unchecked compile-time casts to ButtonNode with a runtime type
guard: import {$isButtonNode} from './ButtonNode' (instead of from
`@tryghost/kg-default-nodes`) and, in ButtonNodeComponent where you call
$getNodeByKey(nodeKey), remove the "as ButtonNode | null" assertions and use if
(!$isButtonNode(node)) return; before mutating or reading node. Apply this
change for all three occurrences (the current casts around $getNodeByKey at the
top of the component and the other two spots where node is assumed to be a
ButtonNode) so runtime instances of the local ButtonNode class are correctly
detected and handled.
| constructor(dataset: Record<string, unknown> = {}, key?: string) { | ||
| super(dataset, key); | ||
|
|
||
| // set up nested editor instances | ||
| setupNestedEditor(this, '__htmlEditor', {editor: dataset.htmlEditor, nodes: BASIC_NODES}); | ||
| setupNestedEditor(this, '__htmlEditor', {editor: dataset.htmlEditor as LexicalEditor | undefined, nodes: BASIC_NODES}); | ||
|
|
||
| // populate nested editors on initial construction | ||
| if (!dataset.htmlEditor) { | ||
| populateNestedEditor(this, '__htmlEditor', dataset.html || '<p>Hey <code>{first_name, "there"}</code>,</p>'); | ||
| populateNestedEditor(this, '__htmlEditor', (dataset.html as string) || '<p>Hey <code>{first_name, "there"}</code>,</p>'); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== EmailNode construction/factory callsites =="
ast-grep --pattern 'new EmailNode($ARG)'
ast-grep --pattern '$createEmailNode($ARG)'
echo
echo "== html/htmlEditor object construction sites =="
rg -nP --type=ts --type=tsx -C3 '\b(htmlEditor|html)\s*:'
echo
echo "== Dataset hydration/export flows =="
rg -nP --type=ts --type=tsx -C3 '\b(importJSON|exportJSON|getDataset)\b'Repository: TryGhost/Koenig
Length of output: 2952
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Get the factory function in EmailNode.tsx to see how it creates the node
echo "== Factory function in EmailNode.tsx (lines around 98) =="
sed -n '90,110p' packages/koenig-lexical/src/nodes/EmailNode.tsx
echo
echo "== EmailPlugin.tsx callsite (around line 20) =="
sed -n '1,35p' packages/koenig-lexical/src/plugins/EmailPlugin.tsx
echo
echo "== Test file examples - how dataset is constructed =="
sed -n '35,50p' packages/kg-default-nodes/test/nodes/email.test.js
sed -n '155,175p' packages/kg-default-nodes/test/nodes/email.test.jsRepository: TryGhost/Koenig
Length of output: 2942
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check BaseEmailNode to see if there's any validation
echo "== BaseEmailNode class definition =="
find . -name "*.ts" -o -name "*.tsx" | xargs grep -l "class BaseEmailNode" | head -1 | xargs cat | head -100
echo
echo "== setupNestedEditor function signature =="
rg -A 10 "function setupNestedEditor|const setupNestedEditor" --type=ts --type=tsx -B 2
echo
echo "== populateNestedEditor function signature =="
rg -A 10 "function populateNestedEditor|const populateNestedEditor" --type=ts --type=tsx -B 2Repository: TryGhost/Koenig
Length of output: 97
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Check EmailNode.tsx for imports and parent class =="
head -30 packages/koenig-lexical/src/nodes/EmailNode.tsx
echo
echo "== Find BaseEmailNode location =="
fd -type f "\.ts$|\.tsx$" | xargs grep -l "class BaseEmailNode" | head -3
echo
echo "== Search for setupNestedEditor implementation =="
fd -type f "\.ts$|\.tsx$" | xargs grep -l "setupNestedEditor" | head -5Repository: TryGhost/Koenig
Length of output: 1523
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Check what setupNestedEditor expects =="
rg "setupNestedEditor" -B 5 -A 15 | head -100
echo
echo "== Check what populateNestedEditor expects =="
rg "populateNestedEditor" -B 5 -A 15 | head -100Repository: TryGhost/Koenig
Length of output: 17308
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Find and read nested-editors utility =="
find . -name "*nested-editor*" -type f
echo
echo "== Check if it exists at the imported path =="
cat packages/koenig-lexical/src/utils/nested-editors.ts | head -150Repository: TryGhost/Koenig
Length of output: 2385
Add runtime type guards for dataset.htmlEditor and dataset.html before passing to helper functions.
Lines 38 and 42 cast unknown values with as without validation. While setupNestedEditor and populateNestedEditor have some defensive checks (if (editor) and if (!html)), they don't fully validate types. A truthy non-LexicalEditor object could be assigned at line 38, causing runtime errors when getEditorState() is later called. Similarly, a truthy non-string value could bypass the falsy check at line 42 and reach generateEditorState.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/koenig-lexical/src/nodes/EmailNode.tsx` around lines 34 - 43, The
constructor currently casts dataset.htmlEditor and dataset.html without runtime
validation before calling setupNestedEditor and populateNestedEditor; add type
guards to validate dataset.htmlEditor is an object exposing getEditorState
(e.g., editor && typeof editor.getEditorState === 'function') before passing it
as the editor option to setupNestedEditor for '__htmlEditor', and ensure
dataset.html is a string (typeof dataset.html === 'string') before passing it to
populateNestedEditor (otherwise pass undefined or a safe default HTML string
like '<p>Hey <code>{first_name, "there"}</code>,</p>'). Update the constructor
logic to use these validated values when calling setupNestedEditor and
populateNestedEditor so generateEditorState / getEditorState won't be invoked on
invalid types.
| export const $createProductNode = (dataset: Record<string, unknown>) => { | ||
| return new ProductNode(dataset); |
There was a problem hiding this comment.
Keep $createProductNode() consistent with the constructor default.
Line 135 makes the exported factory stricter than ProductNode itself, so TS callers now need to pass {} even though the constructor still handles omission. That is an unnecessary public API break.
Suggested change
-export const $createProductNode = (dataset: Record<string, unknown>) => {
+export const $createProductNode = (dataset: Record<string, unknown> = {}) => {
return new ProductNode(dataset);
};🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/koenig-lexical/src/nodes/ProductNode.tsx` around lines 135 - 136,
The exported factory $createProductNode currently requires a dataset argument
while the ProductNode constructor accepts an optional dataset, causing an
unnecessary API break; update $createProductNode to accept an optional parameter
(or provide a default {}) and forward that to new ProductNode so callers can
omit the argument just like when calling ProductNode directly. Ensure the
function signature and implementation for $createProductNode match ProductNode's
constructor optionality.
| if (nodeType && nodeWithUpload.uploadType) { | ||
| acceptableMimeTypes[nodeType] = uploadFileTypes[nodeWithUpload.uploadType].mimeTypes; | ||
| } |
There was a problem hiding this comment.
Guard missing uploadType config before dereferencing .mimeTypes.
uploadFileTypes[nodeWithUpload.uploadType] can be undefined, which will throw at runtime when accessing .mimeTypes. Add a presence check before assignment.
Proposed fix
- if (nodeType && nodeWithUpload.uploadType) {
- acceptableMimeTypes[nodeType] = uploadFileTypes[nodeWithUpload.uploadType].mimeTypes;
+ if (nodeType && nodeWithUpload.uploadType) {
+ const uploadConfig = uploadFileTypes[nodeWithUpload.uploadType];
+ if (uploadConfig?.mimeTypes) {
+ acceptableMimeTypes[nodeType] = uploadConfig.mimeTypes;
+ }
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/koenig-lexical/src/plugins/DragDropPastePlugin.tsx` around lines 60
- 62, The code dereferences uploadFileTypes[nodeWithUpload.uploadType].mimeTypes
without ensuring the lookup exists; update the block in DragDropPastePlugin
(where nodeType and nodeWithUpload.uploadType are used) to first confirm
uploadFileTypes[nodeWithUpload.uploadType] is defined (and that
nodeWithUpload.uploadType is truthy) before assigning to
acceptableMimeTypes[nodeType], e.g. guard the lookup or use optional
chaining/conditional so you only set acceptableMimeTypes[nodeType] when
uploadFileTypes[...] and its .mimeTypes are present.
1 participant
Summary
.js/.jsxfiles to.ts/.tsxacross src/, test/, and demo/tsconfig.json,eslint.config.jswith@typescript-eslint)tseslint.config()withdefineConfig()fromeslint/configEmojifor emoji-mart,SimpleMDEInstance/CodeMirrorInstancefor kg-simplemde)test/types.d.tswith Window augmentation for Lexical editor in e2e browser contexteslint-disablecomments except intentionalreact-hooks/exhaustive-depsfor mount-only effects