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

Add an API for fast HTML escaping #15742

Open
bartlomieju opened this issue Sep 2, 2022 · 7 comments
Open

Add an API for fast HTML escaping #15742

bartlomieju opened this issue Sep 2, 2022 · 7 comments
Labels
suggestion suggestions for new features (yet to be agreed)

Comments

@bartlomieju
Copy link
Member

In various benchmarks that @littledivy and @billywhizz did we see that escaping HTML that a huge chunk of time in React SSR examples.

It's clearly a bottleneck that could be addressed by having a dedicated API for escaping HTML (probably as an op implemented in Rust).

This should allow us to top various React SSR benchmarks.

@bartlomieju bartlomieju added the suggestion suggestions for new features (yet to be agreed) label Sep 2, 2022
@cmoog
Copy link
Contributor

cmoog commented Sep 5, 2022

@littledivy @bartlomieju
For API cleanliness of the top-level Deno namespace, I wonder if it would be smart to use a namespace specifically for these non-WebAPI and non-io operations that are only implemented in native code for performance reasons.

Some potential options

  • Deno.native.escapeHtml
  • Deno.fast.escapeHtml

@kidonng
Copy link
Contributor

kidonng commented Sep 5, 2022

@cmoog While I don't find the idea of stuffing niche APIs under Deno namespace interesting either, those options are worse:

  • Most APIs under Deno namespace are implemented natively so Deno.native is redundant.
  • Deno.fast just doesn't make sense. People expect things to be fast. There's no Deno.slow, nor is anything else under Deno slow.

@cmoog
Copy link
Contributor

cmoog commented Sep 5, 2022

@kidonng

Deno.fast just doesn't make sense. People expect things to be fast. There's no Deno.slow, nor is anything else under Deno slow.

Deno.fast answers the question "Why should I use Deno.escapeHtml vs. a pure JS impl. It's clear why one would use Deno.fast.escapeHtml vs. some other escapeHtml: because it's faster. A few more options

  • Deno.optimized
  • Deno.perf

A pure JS escapeHtml is not fast. As compared to Deno.escapeHtml, it is slow. That's the whole point. Without any semantics that make this distinction apparent to the user, it's confusing what the difference is.

@crowlKats
Copy link
Member

crowlKats commented Sep 5, 2022

A) we don't namespace. the only namespaces we have are the Deno one, and the Deno.errors, and the latter one is an exception of the rule

B)

A pure JS escapeHtml is not fast. As compared to Deno.escapeHtml it is slow.

Web APIs arent implemented in pure JS (except a few that wouldnt make sense otherwise). most of the web APIs have rust code behind them. There is no distinction and it makes no sense to have multiple APIs for the same thing.

@kidonng
Copy link
Contributor

kidonng commented Sep 5, 2022

Without any semantics that make this distinction apparent to the user, it's confusing what the difference is.

Deno namespace is the difference. You can do the same bikeshedding about Deno.test and some other test library.

@cmoog
Copy link
Contributor

cmoog commented Sep 5, 2022

Deno namespace is the difference. You can do the same bikeshedding about Deno.test and some other test library.

Fair. I retract.

@lucacasonato
Copy link
Member

Qwik also has a escapeAttr that may be optimizable the same way: https://github.com/BuilderIO/qwik/blob/8adb568bcb5ea829e266cf75e4fc856dcf93bcf3/packages/qwik/src/core/render/ssr/render-ssr.ts#L826-L837

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
suggestion suggestions for new features (yet to be agreed)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants