[lexical][auto-link] Fix auto link node escapes on second "." #36
Conversation
![local-dev-korbit-ai-mentor[bot]](https://avatars.githubusercontent.com/in/318591?s=60&v=4){kind=small}
![[local-dev] Korbit - AI Mentor](https://avatars.githubusercontent.com/in/318591?s=40&u=4153787aed90096cc5d05c9c38a439007e524086&v=4){kind=avatar}
|
Clone of the PR facebook/lexical#6146 |
|
My review is in progress 📖 - I will have feedback for you in a few minutes! |
WalkthroughThe recent updates enhance the URL handling capabilities of the application. New tests ensure valid URLs are converted to links while invalid URLs remain unaltered, improving overall robustness. Additionally, the URL detection logic has been refined with an updated regex and new functions to handle specific text conditions, allowing for more accurate link creation and processing. Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
![korbit-ai[bot]](https://avatars.githubusercontent.com/in/322216?s=60&v=4){kind=small}
| @@ -13,7 +13,7 @@ import { | |||
| import * as React from 'react'; | |||
|
|
|||
| const URL_REGEX = | |||
| /((https?:\/\/(www\.)?)|(www\.))[-a-zA-Z0-9@:%._+~#=]{1,256}\.[a-zA-Z0-9()]{1,6}\b([-a-zA-Z0-9()@:%_+.~#?&//=]*)/; | |||
| /((https?:\/\/(www\.)?)|(www\.))[-a-zA-Z0-9@:%._+~#=]{1,256}\.[a-zA-Z0-9()]{1,6}\b([-a-zA-Z0-9()@:%_+.~#?&//=]*)(?<![-.+():%])/; | |||
There was a problem hiding this comment.
The modification to the URL_REGEX pattern adds a negative lookbehind assertion (?<![-.+():%]) at the end. While this change aims to prevent premature link creation, it might be too restrictive. Consider testing with a variety of valid URL formats to ensure that this change doesn't inadvertently prevent the recognition of legitimate URLs that end with these characters. You might need to refine the regex further to balance between preventing premature linking and recognizing all valid URL formats.
Chat with Korbit by mentioning @korbit-ai, and give a 👍 or 👎 to help Korbit improve your reviews.
|
/review |
PR Reviewer Guide 🔍(Review updated until commit 24c3000)
|
github-actions
bot
added
Review effort [1-5]: 3
and removed
Review effort [1-5]: 2
labels
|
Persistent review updated to latest commit 24c3000 |
|
@coderabbitai review |
Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- packages/lexical-playground/tests/e2e/AutoLinks.spec.mjs (1 hunks)
- packages/lexical-playground/src/plugins/AutoLinkPlugin/index.tsx (1 hunks)
- packages/lexical-react/src/LexicalAutoLinkPlugin.ts (2 hunks)
Additional comments not posted (5)
packages/lexical-playground/src/plugins/AutoLinkPlugin/index.tsx (1)
16-16: Verify the regex change for broader URL support.The modification to the
URL_REGEXpattern adds a negative lookbehind assertion(?<![-.+():%]). While this change aims to prevent premature link creation, it might be too restrictive. Consider testing with a variety of valid URL formats to ensure that this change doesn't inadvertently prevent the recognition of legitimate URLs that end with these characters.packages/lexical-react/src/LexicalAutoLinkPlugin.ts (2)
92-94: FunctionstartsWithFullStopadded.The function
startsWithFullStopchecks if text content starts with a full stop followed by alphanumeric characters. This addition seems to be aimed at handling edge cases in URL detection.
383-386: Review the updated logic inhandleBadNeighbors.The logic now includes a check for
startsWithFullStop. Ensure this change aligns with the intended behavior of handling URLs and does not introduce unintended side effects.packages/lexical-playground/__tests__/e2e/AutoLinks.spec.mjs (2)
386-467: Comprehensive test coverage for valid URLs.The test case "Can convert URLs into links" covers a wide range of valid URL formats. This is a thorough approach to ensure the correctness of URL conversion logic.
469-515: Comprehensive test coverage for invalid URLs.The test case "Can not convert bad URLs into links" covers various invalid URL scenarios. This ensures that the system correctly identifies and handles invalid URL inputs.
|
Zero comments from coderabbit |
Description
The period in the separators list
PUNCTUATION_OR_SPACEleads to an auto-link creation prematurely before the user completes typing. Removing the "." from the list.Inspired by facebook/lexical#5275 which was reverted because of Regexp changes.
Closes: #5254 #3546
Test plan
Before
Screen.Recording.2024-05-20.at.12.13.38.mov
After
Screen.Recording.2024-05-20.at.12.14.09.mov
Description by Korbit AI
Note
This feature is in early access. You can enable or disable it in the Korbit Console.
What change is being made?
Fix auto-link node escapes on second "." by updating the URL regex and adding tests for valid and invalid URLs.
Why are these changes being made?
The previous implementation failed to correctly handle URLs with multiple periods, causing incorrect link generation. The updated regex and additional tests ensure robust URL detection and conversion, improving the auto-link functionality.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation