[Twig Component] BC break in latest release (2.18)?

[CMH-Benny]

Hey,

Lately symfony ux 2.18 was released and it appears to include a breaking change in it:

Throws if exposed public prop is uninitialized by @smnandre in #1780

Error rendering "FooComponent" component: Cannot expose uninitialized property "$prop" from "App\Twig\Components\FooComponent".

Luckily we only had 2 places that had issue by doing the update, but it was working fine before. It was possible to check for is defined or use the ?? operator to compensate for a non-initialized value. Even tho I see why this change makes sense, but having to deal with it in a minor version update feels not fully right, so I wanted to open this discussion if that is something that should be addressed or not :)

No matter what, thank you for all your efforts on moving forward with Symfony UX, thanks for your effort and passion <3

[CMH-Benny]

Thanks for the fast PR <3
Maybe we can throw a Deprecation instead? That will allow to throw that Exception in the Future on a major version upgrade? WDYT?

But only if that makes sense, was there a specific reason why uninitialized values should be avoided/forbidden? :)

[smnandre]

We ended up here with the idea that properties are a contract exposed by the component, defining what should be "exposed" and what should/could be mounted.

By settings exposePublicVars: true, there is another contract saying "any public property" will be.... exposed.

Undefined properties do not respect any of these.. and this felt a good way to fix that part.

In PHP this triggers a fatal eror

<?php

class Foo {
    public int $bar;
}

(new Foo())->bar;

https://3v4l.org/KuBOn#v8.3.8

--

In my mind, there is no way to ensure properties are mounted while allowing they can not be...
...so I guess we have a Schrodinger's typed property now :)

If anyone want to take the lead on this, please do feel free (and i'm really sincere there, i hope someone find a good solution here)

--

PS: thank you sincerely @CMH-Benny for the positivity and support

[janklan]

That was quick, thanks guys!

In my head, templates are aware of the component's properties anyway, so I was OK with letting the template to decide what to do if schroedingerProperty is not defined. To me, undefined is different from null. If an optional property is not passed to the template, it is undefined, not null.

[ebitkov]

Just to add to the topic:

In my head (and that might be not how it's actually implemented; just saying how I think it could be / how I feel it should be), a public property without a default value (e.g. public string $title;) is required by the component to render correctly and must be set when used. Therefore I would expect an exception, when the value is not set.

On the other hand, a property with an default value (e.g. public bool $invert = false) is optional and some sensible default value is defined, but can be overwritten when used.

---

I had some exceptions too after the update. Most of them came from components where the property was populated while the component was rendering. So he tried to mount those properties (which had no value at this point) and threw the known error. But that was more of an internal structure problem of mine and less something the bundle did wrong.

[norkunas]

a public property without a default value (e.g. public string $title;) is required by the component to render correctly and must be set when used

it could be totally optional if you render things based on other data.

for example I have an Icon component where you can pass name attribute to render from registry, but also you can pass directly the svg contents to it to skip loading from registry, and it broke in the second case.

[smnandre]

I'm reopening the debate here: #1920