-
Notifications
You must be signed in to change notification settings - Fork 654
feat(html/unstable): add escapeJs and escapeCss functions #6782
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
base: main
Are you sure you want to change the base?
Conversation
ebfc40b
to
c998523
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #6782 +/- ##
=======================================
Coverage 94.14% 94.14%
=======================================
Files 589 591 +2
Lines 42540 42564 +24
Branches 6717 6721 +4
=======================================
+ Hits 40050 40073 +23
- Misses 2440 2442 +2
+ Partials 50 49 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
* ); | ||
* ``` | ||
*/ | ||
export function escapeJs(data: unknown, options: EscapeJsOptions = {}): string { |
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.
I don't see why data accepts JSON-serializable object. Shouldn't this be escapeJs(str: string): string
?
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.
Makes it a lot more ergonomic to pass arbitrary(-ish) data:
// might be nested arbitrarily deeply
declare const data: string | number | Record<string, string> | Record<string, Record<string, string>>
const scriptContentToInject = `<script>window.handleData(${escapeJs(data)})</script>`
Or (because the output is also JSON-compatible):
const importMap = { imports: { zod: 'https://esm.sh/v131/[email protected]' } }
const importMapScriptTag = `<script type="importmap">${escapeJs(importMap)}</script>`
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.
Ok. Makes sense. Can you add those as usage examples? Otherwise, I guess users might not get how to use it.
Resolves #6748