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

[DRAFT] Redesign API for trusted (unsafe) strings #20358

Closed
wants to merge 2 commits into from

Conversation

chriskrycho
Copy link
Contributor

@chriskrycho chriskrycho commented Jan 25, 2023

This is a draft PR to accompany a forthcoming proposal to improve this API along lines folks have talked about for years; I drafted the change to make it easier to discuss concretely with the accompanying RFC. This should not be taken as indicating we are doing this, only that I am proposing we do it and making it easy if we decide to do so!


  • Introduce an opaque public type for the result of marking a string as trusted, TrustedString. Make all the mechanical details of the class private, accessible only within the internal module. Define the type as an opaque type that other users can neither instantiate nor subclass, which implements the SafeString contract from Glimmer, i.e. it provides only a toHTML(): string method.

  • Deprecate htmlSafe and isHTMLSafe and introduce new names which indicate what they actually do/are: unsafelyTrustString and isTrustedString.

@chriskrycho chriskrycho force-pushed the htmlSafe-improvements branch from fba0e0c to 81f2d03 Compare January 25, 2023 04:22
This is unused elsewhere in the framework itself and is not public API,
so it should be safe to remove. Additionally, since this is just a copy
of the API from within Handlebars itself, users who want to use it
should directly use Handlebars' (or some other library's) escaping
library instead. See [the discussion at #16817][16817] for background.

[16817]: #16817
@chriskrycho chriskrycho force-pushed the htmlSafe-improvements branch from 81f2d03 to 1127654 Compare January 25, 2023 13:48
- Introduce an opaque public type for the result of marking a string as
  trusted, `TrustedString`. Make all the mechanical details of the
  class private, accessible *only* within the internal module. Define
  the type as an opaque type that other users can neither instantiate
  nor subclass, which implements the `SafeString` contract from
  Glimmer, i.e. it provides only a `toHTML(): string` method.

- Deprecate `htmlSafe` and `isHTMLSafe` and introduce new names which
  indicate what they actually do/are: `unsafelyTrustString` and
  `isTrustedString`.
@chriskrycho chriskrycho force-pushed the htmlSafe-improvements branch from 1127654 to 7d12deb Compare January 25, 2023 14:12
@wagenet
Copy link
Member

wagenet commented Jan 27, 2023

See also emberjs/rfcs#443

@kategengler
Copy link
Member

Is this still relevant? It is a draft PR

@chriskrycho
Copy link
Contributor Author

Still worth doing, and this approach is probably roughly right, but obviously I am not going to land it. I will close this, but/and feel free to point people here as useful prior art, and folks should feel very free to git cherry-pick this and run with it themselves if anyone wants to!

@chriskrycho
Copy link
Contributor Author

…also it needs an RFC. Which is why we did not land it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants