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

Use faster regexp.test over string.match for testing against regex #2053

Merged
merged 1 commit into from
Nov 30, 2023

Conversation

Encephala
Copy link
Contributor

@Encephala Encephala commented Nov 28, 2023

Description

As was mentioned by @xhaggi in this code review, regexp.test is faster than string.match if the goal is to check if a string matches a regular expression. string.match is only necessary if the goal is to extract a substring.

Out of the six places in HTMX where string.match is used, four of them can be replaced by regexp.test.

Corresponding issue: None, suppose that's not necessary for this small change, but correct me if I'm wrong.

Testing

I ran the testing suite and everything came out green.

Checklist

  • I have read the contribution guidelines
  • I have targeted this PR against the correct branch (master for website changes, dev for
    source changes)
  • This is either a bugfix, a documentation update, or a new feature that has been explicitly
    approved via an issue
  • I ran the test suite locally (npm run test) and verified that it succeeded

@Telroshan
Copy link
Collaborator

To back up your proposal, just linking this SO thread where a user posted some performance measurements between those 2

@Telroshan Telroshan added the ready for review Issues that are ready to be considered for merging label Nov 29, 2023
@1cg 1cg merged commit edafdd1 into bigskysoftware:dev Nov 30, 2023
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance ready for review Issues that are ready to be considered for merging
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants