Feat: skip dom purify for trusted content #393
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
yue4u
commented
Sep 25, 2023
| if (iconResult.trusted) { | ||
| this.svgContent = iconResult.svgContent | ||
| } else { | ||
| const { default: DOMPurify } = await import('dompurify') |
There was a problem hiding this comment.
dom-purify is still pretty large so we can do following in the future.
- let users do sanitizing themselves
- leveraging https://developer.mozilla.org/en-US/docs/Web/API/HTML_Sanitizer_API
fsubal
reviewed
Oct 5, 2023
| } | ||
| if (Number.isNaN(size)) { | ||
| throw new TypeError(`icon size must not be NaN`) | ||
| } |
There was a problem hiding this comment.
[IMO] 実は Number.isFinite() だけで良かったりしませんか?
if (!Number.isFinite(size)) {
throw new TypeError('icon size must be a finite number')
}There was a problem hiding this comment.
1/0やInfinityを考えると確かにisFiniteが良さそうです。実際はかなりのedge caseですね
There was a problem hiding this comment.
すみません意図としては typeof が number であるかの判定も isFinite() だけで行けるんじゃないですか、みたいなことでした（これでも良いですが）
There was a problem hiding this comment.
4eb112c 消しました。元々エラー少しわかりやすくなるかもしれないという意図で入れていたが処理的には確か不要そうです
mimokmt
approved these changes
Oct 17, 2024
4 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
やったこと
動作確認環境
チェックリスト
不要なチェック項目は消して構いません