Skip to content

Commit

Permalink
Make the type for SafeString public
Browse files Browse the repository at this point in the history
This type has been effectively "intimate" for many years, and fits in
the same bucket as e.g. `Transition`: it is not user-constructible, but
will be constructed by the framework and users need to be able to name
it. For example, `ember-intl` needs to be able to see that a string has
been marked as trusted to do the right thing to emit it.

Internally, clean up a few long-standing TS issues (`any` etc.), make
`SafeString` explicitly implement the contract from Glimmer so that if
that contract changes, we will know at the definition site, and make
the implementation details of how `SafeString` handles the string it
wraps `private`. (This does not use a `#`-private field because private
class fields have some non-trivial overhead in transpiled contexts, and
`SafeString` can appear in fairly hot rendering paths.)
  • Loading branch information
chriskrycho committed Feb 14, 2023
1 parent 7b11de3 commit 00c69d2
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 13 deletions.
40 changes: 28 additions & 12 deletions packages/@ember/-internals/glimmer/lib/utils/string.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,17 @@
@module @ember/template
*/

export class SafeString {
public string: string;
import type { SafeString as GlimmerSafeString } from '@glimmer/runtime';

export class SafeString implements GlimmerSafeString {
private __string: string;

constructor(string: string) {
this.string = string;
this.__string = string;
}

toString(): string {
return `${this.string}`;
return `${this.__string}`;
}

toHTML(): string {
Expand All @@ -35,10 +37,11 @@ function escapeChar(chr: keyof typeof escape) {
return escape[chr];
}

export function escapeExpression(string: any): string {
export function escapeExpression(string: unknown): string {
let s: string;
if (typeof string !== 'string') {
// don't escape SafeStrings, since they're already safe
if (string && string.toHTML) {
if (isHTMLSafe(string)) {
return string.toHTML();
} else if (string === null || string === undefined) {
return '';
Expand All @@ -49,13 +52,23 @@ export function escapeExpression(string: any): string {
// Force a string conversion as this will be done by the append regardless and
// the regex test will do this transparently behind the scenes, causing issues if
// an object's to string has escaped characters in it.
string = String(string);
s = String(string);
} else {
s = string;
}

if (!possible.test(string)) {
return string;
if (!possible.test(s)) {
return s;
}
return string.replace(badChars, escapeChar);

// SAFETY: this is technically a lie, but it's a true lie as long as the
// invariant it depends on is upheld: `escapeChar` will always return a string
// as long as its input is one of the characters in `escape`, and it will only
// be called if it matches one of the characters in the `badChar` regex, which
// is hand-maintained to match the set escaped. (It would be nice if TS could
// "see" into the regex to see how this works, but that'd be quite a lot of
// extra fanciness.)
return s.replace(badChars, escapeChar as (s: string) => string);
}

/**
Expand All @@ -82,6 +95,7 @@ export function escapeExpression(string: any): string {
@method htmlSafe
@for @ember/template
@param str {String} The string to treat as trusted.
@static
@return {SafeString} A string that will not be HTML escaped by Handlebars.
@public
Expand Down Expand Up @@ -114,6 +128,8 @@ export function htmlSafe(str: string): SafeString {
@return {Boolean} `true` if the string was decorated with `htmlSafe`, `false` otherwise.
@public
*/
export function isHTMLSafe(str: any | null | undefined): str is SafeString {
return str !== null && typeof str === 'object' && typeof str.toHTML === 'function';
export function isHTMLSafe(str: unknown): str is SafeString {
return (
str !== null && typeof str === 'object' && 'toHTML' in str && typeof str.toHTML === 'function'
);
}
4 changes: 3 additions & 1 deletion packages/@ember/template/index.ts
Original file line number Diff line number Diff line change
@@ -1 +1,3 @@
export { htmlSafe, isHTMLSafe } from '@ember/-internals/glimmer';
// NOTE: this intentionally *only* exports the *type* `SafeString`, not its
// value, since it should not be constructed by users.
export { htmlSafe, isHTMLSafe, type SafeString } from '@ember/-internals/glimmer';

0 comments on commit 00c69d2

Please sign in to comment.