codeant-ai-for-open-source[bot] commented on PR #36417: URL: https://github.com/apache/superset/pull/36417#issuecomment-3609847721
## Nitpicks 🔍 <table> <tr><td>🔒 <strong>No security issues identified</strong></td></tr> <tr><td>⚡ <strong>Recommended areas for review</strong><br><br> - [ ] <a href='https://github.com/apache/superset/pull/36417/files#diff-5167982faf423b683427b7adbe1043d554f4cfcc11124acd514c38e63c3e6c77R181-R197'><strong>Enter-key selection bug</strong></a><br>The Enter-key handling logic appears inverted: the code checks for lastSearchRef.current.length === 0 when deciding to treat a quick Enter as newline. This contradicts the comment and likely allows accidental selection instead of preventing it. Verify and correct the condition so a quick Enter after typing an emoji search prevents selection (i.e., check for presence of meaningful search text or meet minCharsBeforePopup).<br> - [ ] <a href='https://github.com/apache/superset/pull/36417/files#diff-7bfa9472e219f53d45a5034d6ef1e3b895d1dfabe1e3b375e8e98665620e23c4R105-R111'><strong>Verification of source exports</strong></a><br>Ensure `./EmojiTextArea` actually exports all named items listed here (`EmojiTextArea`, `filterEmojis`, `EMOJI_DATA`) and the type names (`EmojiTextAreaProps`, `EmojiItem`). Missing or renamed exports will break builds that import from this barrel.<br> - [ ] <a href='https://github.com/apache/superset/pull/36417/files#diff-5167982faf423b683427b7adbe1043d554f4cfcc11124acd514c38e63c3e6c77R26-R28'><strong>Emoji regex limitations</strong></a><br>The EMOJI_REGEX is a simplified range-based pattern that may not cover all emoji sequences (multi-codepoint emojis, skin-tone/modifier sequences, ZWJ sequences, etc.). Consider if incomplete matching could cause incorrect trigger behavior when previous character is part of a complex emoji.<br> - [ ] <a href='https://github.com/apache/superset/pull/36417/files#diff-5167982faf423b683427b7adbe1043d554f4cfcc11124acd514c38e63c3e6c77R90-R126'><strong>Trigger detection may match wrong colon</strong></a><br>The implementation uses fullValue.lastIndexOf(`:${text}`) to find the colon for the current search, which can match an earlier colon elsewhere in the text rather than the colon at the caret. This may cause the popup to trigger incorrectly (or miss triggers) when multiple colons exist. Use the cursor/selection position to ensure the matched colon is the one immediately preceding the caret.<br> - [ ] <a href='https://github.com/apache/superset/pull/36417/files#diff-f3b950dda3ac596441153ec7be33a2b1bc585d2cf4541f41ed387b30c72d0c1fR555-R569'><strong>Search parsing</strong></a><br>`filterEmojis` treats the provided `searchText` raw. If callers pass values that include a leading colon (e.g., ":sm"), or whitespace, the function will include the colon in the search and may return unexpected results. The PR description expects popup triggers on ':' and a minimum of characters before showing suggestions (minChars). The filter function does not trim leading ':' nor enforce a minimum character count, which can lead to mismatch with component behavior.<br> - [ ] <a href='https://github.com/apache/superset/pull/36417/files#diff-f3b950dda3ac596441153ec7be33a2b1bc585d2cf4541f41ed387b30c72d0c1fR31-R550'><strong>Bundle size</strong></a><br>The entire emoji catalog (EMOJI_DATA) is inlined in this module (~500+ entries). That will increase the package/bundle size and cause the emoji list to be loaded whenever this module is imported, even if emojis are rarely used. Consider lazy-loading the data, moving it to a separate JSON file, or providing an async loader to avoid increasing initial bundle size and CI artifact sizes.<br> - [ ] <a href='https://github.com/apache/superset/pull/36417/files#diff-7bfa9472e219f53d45a5034d6ef1e3b895d1dfabe1e3b375e8e98665620e23c4R105-R111'><strong>Bundle-size risk</strong></a><br>Re-exporting `EMOJI_DATA` (a large runtime constant with 400+ emoji entries) from the central components barrel can increase consumers' bundle sizes if they import from the barrel. Consider lazy-loading the data or re-exporting only types from barrels to avoid pulling heavy runtime data into unrelated bundles.<br> - [ ] <a href='https://github.com/apache/superset/pull/36417/files#diff-b77c4599957634ca52005064799b78f16881f2df4422e5a995dcbe40afa81b58R150-R154'><strong>Index key usage</strong></a><br>The mapped list of selected emojis uses the array index as React key which can lead to incorrect DOM reconciliation and visual glitches when items are inserted/removed. Use a stable unique key (e.g., shortcode or id) instead of the index.<br> </td></tr> </table> -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
