-
-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Improve escape_html_attr
performance
#4024
Conversation
'"': '"' | ||
}; | ||
|
||
const escape_html_attr_regex = new RegExp( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we add a comment about why this is needed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why delete < and >
they break HTML if not escaped
we had issue #3773
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@PH4NTOMiki That was about escape_json_in_html
, this is about escape_html_attr
.
@benmccann Sorry about the comments, the previous code didn't have them and I thought they weren't necessary. I'll add some.
c6de435
to
4514809
Compare
|
4514809
to
4d146d0
Compare
Nice, I did a naive benchmark locally and this is way faster |
Closes #4016.
Depends on #4015.
Replaces the character-by-character loop with a regex-based
str.replace
call.Please don't delete this checklist! Before submitting the PR, please make sure you do the following:
Tests
pnpm test
and lint the project withpnpm lint
andpnpm check
Changesets
pnpx changeset
and following the prompts. All changesets should bepatch
until SvelteKit 1.0