Ember.String.isHtmlSafe RFC

[workmanw]

Rendered.

More details about this RFC can be found: emberjs/ember.js#13318

Polyfill implementation: https://github.com/workmanw/ember-string-ishtmlsafe-polyfill

[stefanpenner]

I don't believe instanceof is the right thing here. Rather if htmlSafe is a function

[mixonic]

i.e. I think, thought you were talking about the browser :-)

[workmanw]

Hehe. Yea I should clarify that. Thanks!

[rwjblue]

@workmanw - This looks good! Since we will be removing the deprecation that was added in 2.6.0-beta.1 can you add that deprecation as part of this RFC? When we add it back, we will make sure that it still passes a str instanceof Ember.Handlebars.SafeString check (to prevent the original issue).

[workmanw]

@rwjblue Yea, absolutely.

[stefanpenner]

I would be fine with isHtmlSafe being a function that returns true. It is likely more ergonomic then the typeof check.

alternatively and likely a better choice would be to have SafeString.isHtmlSafe(any) that way the user doesn't need any guards or anything, also it decouples the implementation of branding from the identification of the branding. Which seems quite future proof

[rwjblue]

I would be fine with isHtmlSafe being a function that returns true. It is likely more ergonomic then the typeof check.

Yes, that is what is being proposed here.

alternatively and likely a better choice would be to have SafeString.isHtmlSafe(any) that way the user doesn't need any guards or anything

How would this work? The proposal here suggests adding Ember.String.isHtmlSafe(<any>) that returns true / false. We would ultimately remove Ember.Handlebars.SafeString (adding a deprecation to Ember.Handlebars.SafeString is what drummed up the need for a public API to detect if a given object is an HTML safe string).

it decouples the implementation of branding from the identification of the branding

Hmm, the go forward path for operating with HTML safe strings is two methods on the same object:

• Ember.String.htmlSafe(someString) -- This generates an HTML safe string.
• Ember.String.isHtmlSafe(<any>) -- This detects if a given thing is a safe string (and is what this RFC is proposing).

This seems exactly like you are describing, what am I missing?

[stefanpenner]

@rwjblue the implementation

rather then isHtmlSafe doing the instanceof check, it checks the branding. Note for security reasons the branding must be x && typeof x.someMember === 'function'

[rwjblue]

@stefanpenner - Yep, I commented similarly inline here (we should definitely not be doing instanceof to check this). IMO, the implementation of how to determine if a given thing is an HTML safe string is not part of this RFC (though obviously it will need to be done at implementation time), and we should definitely be doing the function check vs instanceof.

[stefanpenner]

@rwjblue sounds like we are in agreement, my mobile client doesn't do a good job of letting me do line comments.

[rwjblue]

@stefanpenner - Gotcha, sorry for talking past each other there! Yes, I believe we are both on the same page. This RFC has been updated to mention the typeof obj === 'function' check (instead of instanceof), and now talks about the deprecation path. I think it is ready for a re-read...

[workmanw]

Updated the RFC with the proposed implementation change. Also put together a polyfill that uses the "branding" technique that Stefan described. ember-string-ishtmlsafe-polyfill.

[mixonic]

@workmanw thanks for this, and sorry this has hung here for so long. I'll push for some approval this weekend or this Friday and hopefully we can get it rolling :-D

[workmanw]

Thanks! Happy to submit a PR for implementation given how trivial this is it shouldn't take much time at all.

[rwjblue]

Discussed during core team F2F, and we believe this is good for implementation.

@workmanw - Would you mind working on implementation in Ember and creation of a polyfill once the Ember implementation PR is landed?

[workmanw]

@rwjblue Happy to work on the implementation. A polyfill already exists :). https://github.com/workmanw/ember-string-ishtmlsafe-polyfill . But I will improve this polyfill once the implementation lands.

[rwjblue]

👍