Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rewrite LOOSE_URL_WEBSITE_REGEX #822

Merged
merged 1 commit into from
Dec 4, 2024

Conversation

s77rt
Copy link
Contributor

@s77rt s77rt commented Dec 2, 2024

This rewrite is needed for better performance as the previous regex caused too many backtracks that hermes was not able to handle

Fixed Issues

$ Expensify/App#52475

Tests

Existing tests are enough

QA

None

@s77rt s77rt requested a review from a team as a code owner December 2, 2024 21:19
@melvin-bot melvin-bot bot requested review from AndrewGable and removed request for a team December 2, 2024 21:19
@AndrewGable AndrewGable requested review from dangrous and removed request for AndrewGable December 2, 2024 21:21
@dangrous
Copy link
Contributor

dangrous commented Dec 3, 2024

@ntdiary can you take a look at this one? I can't assign you as a reviewer on GH since it's expensify-common

Copy link

@ntdiary ntdiary left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ntdiary can you take a look at this one? I can't assign you as a reviewer on GH since it's expensify-common

@dangrous, I've tested the new regex in the live markdown example, and it’s working great. :)

regexp.mp4

@dangrous dangrous merged commit eb0dfcd into Expensify:main Dec 4, 2024
6 checks passed
@os-botify
Copy link
Contributor

os-botify bot commented Dec 4, 2024

🚀 Published to npm in 2.0.108 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants