Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Typed persistent parameters #230

Closed
jtojnar opened this issue Sep 11, 2019 · 3 comments
Closed

Typed persistent parameters #230

jtojnar opened this issue Sep 11, 2019 · 3 comments

Comments

@jtojnar
Copy link

@jtojnar jtojnar commented Sep 11, 2019

Currently, the components’ persistent parameters are only converted to a type when they have default value:

$type = gettype($meta['def']);
if (!$reflection->convertType($params[$name], $type)) {

Sometimes, the value can only be set dynamically but we would still like for it to be properly converted in loadState.

I propose recognizing typed annotations like @var int @persistent

It would need to be extracted here and then handled in the function above.

I can implement this if you agree.

jtojnar added a commit to jtojnar/VisualPaginator that referenced this issue Sep 11, 2019
Nette only recognize persistent params as typed when they have default value.

nette/application#230

Since users are allowed to set various static properties after the construction of VP,
we cannot set the default values and need to convert the params manually.
@dg

This comment has been minimized.

Copy link
Member

@dg dg commented Sep 13, 2019

Can you better describe what situation you want to deal with?

@jtojnar

This comment has been minimized.

Copy link
Author

@jtojnar jtojnar commented Sep 13, 2019

The VisualPaginator component has two persistent properties: $page and $itemsPerPage. In loadState method, they are assigned to the corresponding properties of Nette\Utils\Paginator. With strict_types on, this fails unless we manually convert the properties to int, which, while being a trivial modification, feels like something that our framework could spare us doing.

And indeed, it does convert the type for us when we set a default value. However, that is not always possible. We can assign the default value 1 for the $page property statically but the default value of $itemsPerPage depends on another property, $itemsPerPageList, which can be modified after the construction of VisualPaginator. In that case it is not right to set default value to some dummy number because then, the component could be internally inconsistent ($itemsPerPageList[$itemsPerPage] needs to exist).

Setting the type to ?int, Nette could easily convert the value of the persistent property in the background like it does with properties that have a default value set, allowing us to clean up the code slightly.

@dg

This comment has been minimized.

Copy link
Member

@dg dg commented Sep 13, 2019

I understand, you just need nullable typed property.

I will add them, but I wouldn't probably support the @var annotations, but I would add support for type properties in PHP 7.4

@dg dg closed this in d2e8e63 Sep 13, 2019
dg added a commit that referenced this issue Sep 18, 2019
dg added a commit that referenced this issue Sep 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.