what are the requirements for the view_template method - to_str is not used

[davetron5000]

Hi, first time using Phlex. I'm trying to use it to replace a custom-created view layer. Part of the custom view layer is a class called HTMLSafeString, which wraps a string and thus indicates its contents don't need to be escaped. It implements both to_s and to_str. I realize in a Phlex-powered app, this class isn't needed, but it's still being used as a try out Phlex and transition.

In my Phlex component, I am doing something like this:

def view_template
  div do
    items.map do |item|
      span { t("messages.#{item.key}") }
    end
  end
end

In this case t is a method I created that returns an HTMLSafeString. For the sake of the question, let's assume that this object's to_str method returns the String "The message that t returns".

When I call call, Phlex returns <div><span></span></div>

If I change my code to do t("messages.#{item.key}").to_s, Phlex returns <div><span>The message that t returns</span></div> (which is the desired output).

These leads to a few questions:

• What are the allowed return values from view_template? Obviously String is, and presumably whatever Phlex's DSL methods return.
• If some types aren't allowed, should Phlex raise an error instead of rendering nothing and/or can Phlex be configured to raise an error at least in dev/test?
• to_str is how one indicates an object can be used as a string - should Phlex call this?

[davetron5000]

OK, some context from looking at the code. It seems that a few pieces of code contribute to this behavior:

https://github.com/phlex-ruby/phlex/blob/258a2de4f4ac64abfeb0d0b9f2395a4cc6965d3f/lib/phlex/sgml/elements.rb#L35-L49

Code like it appears in a few places, but the gist is that if the return value is not SafeObject, String, or Symbol, it calls format_object

https://github.com/phlex-ruby/phlex/blob/258a2de4f4ac64abfeb0d0b9f2395a4cc6965d3f/lib/phlex/sgml.rb#L364-L369

Which returns nil if the object is not a Float or Integer

There are some places where code like the first link does check if it gets nil back and raises an error, but sometimes not. It seems like the "check the type of the return value" code is duplicated in a lot of places, so it makes sense that it's not always checked.

I can see a few possible changes, but this is my first time looking at this code, so I have no idea if any of this makes sense:

• format_object could call to_str if it's defined instead of returning nil
• Phlex could adopt it's own "you have my permission to treat me as a string" protocol and call that in format_object
• format_object could raise an error instead of returning nil (or maybe only do that based on some opt-in configuration setting
• The code that checks the type of the return value could be consolidated into one place and centralize error handling there (seems like a big change, but maybe better in the long run?)

[joeldrapper]

I need to get back to a laptop to read your question fully but the short answer is that Phlex doesn't call to_s because too many objects implement that including Enumerators which could lead to unintended output. For example if you used each to iterate over an array but didn't output anything in the loop, Phlex would attempt to render the enumerator calling to_s on it.

The format_object method is designed to be overridden. You can add your own logic and call super.

Alternatively, you could make your t method return a string.

[joeldrapper]

Note, this solution is not for HTML safe strings. Phlex will still HTML-escape whatever you return from format_object so it’s not the right solution to your problem.

[joeldrapper]

Okay, back on a laptop. Since you’re talking about an HTMLSafeString that needs to be output as raw without being escaped, there are two options:

Safe value

You can create a Phlex::SGML::SaveValue object, passing in the string.

Phlex::SGML::SafeValue.new("my safe string")

There’s a shortcut to do this from inside a Phlex component using the safe method.

safe("my safe string")

Phlex::SGML::SafeValue implements the Phlex::SGML::SafeObject interface, which you can implement yourself.

Safe Object Interface

Phlex::SGML::SafeObject is a module you can include into any class to declare that its to_s method returns a string that’s safe to output in SGML contexts (HTML/SVG).

class MySafeHTMLString
  include Phlex::SGML::SafeObject

  def initialize(value)
    @value = value
  end

  def to_s
    @value
  end
end

This module is how phlex-rails makes phlex treat ActiveSupport::SafeBuffer as a safe object.

[davetron5000]

Sorry, I think I made the question slightly confusing based on my class - this isn't a question about HTML escaping or raw HTML (how Phlex does that is clear from the docs 👍). It's more what I think is a bug around type handling of view_template and a possible fix.

Bug - Phlex renders nothing on unsupported return type from view_template and friends

It appears that if view_template or any block given to a DSL method returns an object other than String, Symbol, Float, Integer, or Phlex::SGML::SafeObject, Phlex will not raise an error nor log a warning, and instead render an empty string. This seems less than ideal - it would be a better experience if an error or warning were raised.

Feature That Could Fix Said Bug - Using to_str

Your logic around to_s is sound and makes sense. But I'm talking about to_str, which is Ruby's protocol for implicit string conversion. It's implemented by objects that want to advertise that they can be safely treated as a String. You've probably seen this error:

irb(main):003> puts "We got" + x
(irb):3:in 'String#+': no implicit conversion of false into String (TypeError)

Annoying, because false.to_s is "false", however in this case, it's using to_str, which FalseClass does not implement.

And, for any object passed to a method that requires an opting-in to string conversion, you'd get the same issue. But, you can tell Ruby that you are OK with a string conversion by implementing to_str:

class MyClass
  def initialize(value)
    @value = value
  end
  def to_str = @value.upcase
end
=> :to_str
irb(main):013> x = MyClass.new("foo")
=> #<MyClass:0x0000ffff6be26290 @value="foo">
irb(main):014> puts "we got " + x
we got FOO

Phlex's format_object could, instead of implicitly returning nil, call to_str. This would be consistent with the intentions of the method, and not inadvertently convert objects that should not be converted to strings, since it is an opt-in protocol. It would also address the bug above - in my case, my proprietary string would've been rendered, but in other cases where say the wrong type was used, an error would be raised.

[joeldrapper]

I agree format_object should probably call to_str. I can’t remember why it doesn't do that already.

[davetron5000]

As I use the library more, there could be more places to call to_str - in my app, I deal with URI instances for URLs, so I have stuff like a(href: some_uri) and it fails since href: expects a string. Since URI implements to_str it could be automatically converted.

That said, I don't want to scope creep this discussion :)

[joeldrapper]

I’m going to do some digging through old issues to try to find where we discussed this before. It’s possible we ran into a blocker. I know we specifically don’t call to_s because it often leads to unexpected behaviour. Calling to_str seems much safer.

[davetron5000]

A way around this that @joeldrapper may have mentioned in Discord is to include Phlex::SGML::SafeObject and make sure to_s does the right thing. SafeObject is marked private, but it could be "promoted" to public to serve exactly this purpose.

That said, I can see Phlex maybe wanting to have a private class for understanding what is safe and a different API for users. A few thoughts:

• Phlex::SGML::SafeStringConversion could be the public version of the SafeObject module, perhaps using a phlex-specific method to avoid accidental use, e.g. to_safe_s or something.\
  • Pros: explicit, doesn't conflict with Phlex internals, can be checked in a case
  • Cons: Duplicative of existing SafeObject and to_str
• Eschew a module entirely and rely on to_str as discussed.
  • Pros: A more "standard ruby" way of understanding how to turn something into a string
  • Cons: Requires a check e.g. respond_to?(:to_str) in order to provide a good error message, "treating an object as a String" and "this string is safe to put into HTML" aren't exactly the same thing, so could be considered an overload of to_str
• Make Phlex::SGML::SafeObject an official public module/API
  • Pros: Smallest change
  • Cons: Harder for Phlex to separate internal safety from user-indicated safety, relying on to_s could create inadvertent conversions of SafeObject is widely used in a deep class hierarchy.

[davetron5000]

One other thing in favor of to_str that I forgot is that some classes out of my control implement this, namely URI. In my app, I use URIs instead of Strings to manage, well, URIs :) Thus, I have this sort of thing:

a(href: WidgetsPage.routing.to_s)
#                           ^^^^

My current solution is to call extend(Phlex::SGML::SafeObject) on URI instances returned from my app's innerworkings. This works, but I'd obviously prefer to not have to do that.

[joeldrapper]

I do think we should probably support to_str. I just need to see if I can find any examples that might cause an issue. The main one is Eumerators and they don’t respond to to_str so it should be fine.