FEATURE: Support objects that implement __toString in string propType

[mficzel]

This change replaces the default flow string validator that only accepts null or strings with a custom one that will also accept objects that implement __toString.

Among other things this will be allowed by this:

• i18nTokens without calling translate in the end
• kaleidoscopeImageSources can be passed to string properties ... and will render as uri

Resolves. #13

[mficzel]

I am not entirely sure about this but this would at least resolve the problems that translation tokens are rejected from the string propType unless translate() is called explicitly.

[grebaldi]

I'm not sure on this one either. At first glance, this makes perfect sense. But there's a problem on the PHP side of things.

PHP 8 does elaborate on this concept by introducing the Stringable interface: https://www.php.net/manual/en/class.stringable.php

Yet, even in PHP 8 string and Stringable are not the same thing in a declare(strict_types=1); environment and would need to be explicitly typehinted via union: string|Stringable.

The suggested design of PropTypes.string would lead to a false contract by suggesting that the validated prop could be passed to Eel-Helper methods with a string parameter TypeHint. But all values that are actually just objects implementing __toString won't pass that TypeHint.

I have two suggestions to remedy this:

1. In PHP 8, if you use a string|Stringable TypeHint on a method signature, the parameter will implicitly be converted to a string. That's a pretty weird design choice, that I do not really understand. But, since PHP is doing it this way, it might be worth considering that PropTypes does the same. It would be possible, but it would also necessitate that PropTypes are available in Production contexts - so I'd consider it a bad option after all.

2. Introduce PropTypes.stringOrStringable. This might be cumbersome to write on the consumer's end and would probably cause a bit of confusion over when it should be preferred over PropTypes.string. But contract-wise it would suggest the correct ideas:

• This value will be printed and concatenated correctly as a string
• Better use String.toString before passing it to an Eel-Helper method

Wdyt?

[mficzel]

@grebaldi not sure wether we should model propTypes after php as this is "just" the language it is written in. I would rather reflect about wether or not we consider it ok from a fusion / component perspective.

I kind of find it annoying that code that does exactly what it is supposed to do in production is currently rejected in development.

Let us think about what can go wrong for a minute: The only case i think this might lead to errors is when components accept string proptypes but later use object methods. This is a bit exotic to me and i am not sure i want to have extra code for that.

[markuspfeifenberger]

To keep things simple, I would appreciate the simple solution validate PropTypes.string as true, if it is an object and the __toSTring()function exists.

The case, that someone defines a prop to be a string and later use is as object is kind of a wrong definition. If the object ist used later on, the corresponding type should be used instead of PropTypes.string

[markuspfeifenberger]

This pull request works without errors

[grebaldi]

@markuspfeifenberger:

The case, that someone defines a prop to be a string and later use is as object is kind of a wrong definition. If the object ist used later on, the corresponding type should be used instead of PropTypes.string

The problem isn't so much the prop being used as an object later on. It's more that PropTypes.string would suggest you're dealing with an actual string, but an object that implements __toString isn't actually a primitive string at all.

Imagine you define a prop as PropTypes.string and you pass it to another Component. There it gets passed to yet another Component and so on... Now let's say that three levels down you'll pass the prop to an Eel-Helper that takes a string argument. In a declare(strict_types=1); environment, you'd get into trouble now, because your prop isn't considered to be a string in PHP and passing it to the Eel-Helper will throw a TypeError.

Admittedly, most built-in Eel-Helpers aren't that strict and most string-related methods even try to do an implicit conversion. But there's other trouble awaiting even with the built-ins, like:

prototype(Vendor.Site:MyComponent) < prototype(Neos.Fusion:Component) {
    @propTypes {
        label = ${PropTypes.anyOf(PropTypes.string, PropTypes.integer)}
    }

    renderer = afx`
        <button>{Type.isString(props.label) ? "It's a string" : "It's a number"}</button>
    `
}

Type.isString(props.label) is going to evaluate to false for an object that implements __toString.

These are just examples off the top of my head - I'm sure there's even more problems that are not quite forseeable right now.

Theoretically, we could just document that PropTypes.string will let __toStringables pass and then warn about the consequences. But what if a user really wants to make sure that only an actual primitive string will pass? We'd need another API for that.

I'm sorry, but I have to reiterate my objection :( It'd be best to keep the behavior of PropTypes.string as it is. Users can easily convert __toStringables via String.toString before they're being passed to presentational components. Enabling the convenience to leave out that step would result in a mess.

[markuspfeifenberger]

@grebaldi I got you. So a way to handle this will be to add a new Proptype. I would suggest PropTypes.stringable - evaluation to true, if the props has the __toString function or is already a string.

Additionally the exception for the PropTypes.string can be extended. In the case, that the _toString exists, an info text to either convert with String.toString or use PropTypes.stringable instead can solve the exception.

This as @mficzel said, would solve the translation problem as well.

Wdyt?