phlex
phlex copied to clipboard
`to_str` vs `to_s`
So at https://github.com/phlex-ruby/phlex/blob/main/lib/phlex/sgml.rb#L422 to_str is used, but not all objects respond to that method. Think using to_s is better.
As an example, ActiveSupport::TimeWithZone does not implement to_str, but does to_s, resulting in an exception when using a ActiveSupport::TimeWithZone instance as an attribute...
time(datetime: record.updated_at) { record.updated_at.to_fs :short }
undefined method 'to_str' for Wed, 23 May 2018 14:59:48.000000000 UTC +00:00:ActiveSupport::TimeWithZone
Hey @joelmoss, this was intentional because to_s is usually used to represent an object for inspection. All kinds of objects respond to to_s, sometimes in unexpected ways — even things like Enumerators and Data objects.
For this use-case, you could define format_object in your base ApplicationComponent to handle ActiveSupport::TimeWithZone and format it with your preferred strftime representation.
def format_object(object)
case object
when ActiveSupport::TimeWithZone
object.strftime("%H:%M on %d %B, %Y")
else
super
end
end
This method is designed to be overridden like this with rich object formatters.
Sorry, I got this wrong. The format_object method is only called by Phlex for text output, but I realised you're talking about attribute values.
Yeah, I'm talking about attributes.
Regarding attributes, I can see why this is frustrating, but on balance, I do think it's better to use to_str rather than to_s. So many objects respond to to_s in ways that are not designed for public output. If the objects can really be coerced to strings without any additional thought or parameters, they should respond to to_str.
In the case of TimeWithZone, there's no universal way to represent date-time-with-timezones as strings, so they should probably be converted into strings with strftime, which importantly takes formatting parameters.
The same is true of other rich objects such as Money. If it's a data attribute to be consumed by JavaScript, you probably want to represent Money as an integer of cents (e.g. 1000); if it's for display somewhere, you probably want a formatted string (e.g. $10.00). Phlex can't know this, so it's better to error until you specify.
One final example is ActiveRecord objects and other models: if I have an model (e.g. user), it responds to to_s with:
"#<User:0x000000012733b378>"
It could have been represented like this:
'#<User id: 6, email: "[email protected]", role: "admin", first_name: "Joel", last_name: "Drapper">'
Or perhaps as an identifier:
6
Or a global identifier:
"gid://example/User/6"
Or a URL:
"/users/6"
Or JSON:
{
"id" : 6,
"email" : "[email protected]",
"role" : "admin",
"first_name" : "Joel",
"last_name" : "Drapper"
}
Or a name:
"Joel Drapper"
I don’t think there's a sensible default for many objects besides the currently supported String, Symbol, Integer, Float, true, false, nil, to_str-able, Arrays of the above types and Hashes with String|Symbol keys and values of the above types.
That's actually really interesting, as it seems we both have a different view on #to_s and what it is generally used for. It seems that you would use it for technical reasons, whereas I really only ever use it for end-user display purposes, or as you put it, for public output.
For example in pretty much every model or class I have, I would define a to_s method that would return a user friendly way of identifying the object. So a User model with a name method/attribute would be returned from, or simply aliased to to_s. Then anything else would use a more specific method, such as to_json, to_url, etc.
But the real issue here - at least for me - is that #to_s is more universally used than to_str in Ruby land. So it makes more sense to use that
There are a few articles on this topic:
- https://blog.appsignal.com/2018/09/25/explicitly-casting-vs-implicitly-coercing-types-in-ruby.html
- https://www.rubyguides.com/2018/09/ruby-conversion-methods/
Essentially, every Ruby class (with the exception of BasicObject) implements to_s, either as the default object inspect representation or with a specialised implementation. to_str, on the other hand, is usually only implemented by objects that can be coerced into a string without data loss, or where it makes sense for the object to be treated as if it were itself a string.
I think this is the interface we're looking for.
With a TimeWithZone attribute, I would define a private method on the component to format it into a string.
def template
div(data_date: date)
end
def date = @date.strftime("%H:%M on %d %B, %Y")
With your own POROs, it may make sense to just implement to_str. For example, if you have a simple value object like UserID, it may be appropriate for that to be to_str-able.
UserID = Data.define(:value) do
def to_str = case value
in Integer then value.to_s
in String then value
end
end
Sorry to bring this up again, but this keeps cropping up over and over for me, and having to implement #to_str on every object is becoming a real pain.
It's even more frustrating that only String in the standard library implements #to_str, and even that is an alias to #to_s. It wouldn't be too bad if most implemented it, but hardly any do.
While I understand the reasonings behind preferring to_str, in reality very little code uses it.
Is there some sort of compromise we can find here?
We could allow text to call to_s on the object. I don’t like the idea of using to_s for implicit text (from the block return) because every object in ruby implements to_s, including things like Enumerators.
I agree with not using it for implicit text, so I think some sort of convention is needed so as to avoid me having to implement #to_str everywhere.
We could allow text to call to_s on the object
Could show an example?
Could show an example?
What I mean is this wouldn't call to_s on the object.
h1 { object }
But this would
h1 { text object }
Yeah, but I'm talking about attributes remember 😏
🤦♂️ Oh god, I keep doing this.
I guess it makes sense to call to_s on objects passed as attributes, though I’d prefer an interface that allows you to have an object that can be coerced into other valid attribute types such as true or false. For example, it could try to_phlex_attribute_value first and then fall back to to_s.
Yeah I like that idea.
I'm guessing that would be best implemented at https://github.com/phlex-ruby/phlex/blob/main/lib/phlex/sgml.rb#L422
But that currently calls #to_str. So do we try to_phlex_attribute_value, to_s, to_str (in that order)?
I’d prefer to try to_str before to_s, but it's probably better for performance to call to_s first since — let's be honest — almost no object implements to_str. 😅
"upgrading" (the geese are there to signal that it certainly does not feel so) Phlex now hands me
ActionView::Template::Error (undefined method `to_str' for Location:Class
first I went hunting some of my own 'darlings' until I realized - when trying out endpoints not touched since like forever, that this upgrade really has "paid off"
- SVG now works with a block [only] (not a biggie but it cheated me off a good 20min)
- text is now plain (neither a biggie - but another 10-15min)
- attributes on Phlex inherited classes now somehow has their to_s/to_str all screwed up
- turbo streaming suddenly stopped working
- probably more
I'm (as you may remember) a huge fan of Phlex - the DSL, the bang and all the fizz - but you might put up a (not so small) sign warning current users:
Road bump ahead
√ src % bundle list
Gems included by the bundle:
* actioncable (7.0.7.2)
*...8<...
* phlex (1.8.1)
* phlex-rails (1.0.0)
* ...8<...
Thanks for the feedback @wdiechmann. All of these changes were made after very careful consideration and documented in the release notes.
Turbo stream shouldn't have stopped working though. Could you share a bit more about that in a new discussion thread? We actually managed to get some changes into turbo-rails itself that makes it work even better with Phlex views and other renderable objects like ViewComponent components.
Also, I’m sorry the upgrade wasted 35 minutes of your time. I’ve probably spent over eighty thousand minutes working on phlex and phlex-rails so far so hopefully on the whole it's saved you a little time too.
I understand that my 'ventilating discomfort' came out all wrong - apologies!
I'll hurry to the release notes while I enjoy all the time I've saved using Phlex!
If you ever feel adventurous please ping me - I've got 1-2 places for you to have a well-deserved R/R 😄
guess I'll stay with 1.3.2 for the time being though...
@joelmoss want me to assign this issue to you or shall I take it? Happy either way.
I'll be very happy to volunteer testing this issue in a 'real' project
@joelmoss want me to assign this issue to you or shall I take it? Happy either way.
I was planning on grabbing it. Just need to find the time. 😕
So it turns out that when trying to_phlex_attribute_value, to_s, to_str in order, the to_str will never get called, because Object implements to_s. Meaning every object at the very least respond to to_s.
So we either switch the order to to_phlex_attribute_value, to_str, to_s, or simply don't try to_str.
Thoughts?
FYI: draft PR https://github.com/phlex-ruby/phlex/pull/629