Skip to content

[FEATURE] Allow variables assigned with dotted path #425

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

Conversation

NamelessCoder
Copy link
Contributor

Enables assignment of template variables with a dotted
path as name:

$view->assign(‘parent.property’, ‘newValue’);

Each value in the path leading up to the final property
name is created as an array if it does not exist, and is
read by reference until a subject is identified. Then the
value is set on that subject.

If a path points to an object and the path to the property
contains one or more scalar values, assignment is
refused with an exception clearly stating why a subject
could not be resolved.

The feature is also enabled for the JsonVariableProvider
by making it call the StandardVariableProvider’s method.

@NamelessCoder NamelessCoder force-pushed the feature/assign-and-override-with-dotted-path branch from d8d2611 to a2d4b14 Compare January 7, 2019 17:26
@TYPO3 TYPO3 deleted a comment from coveralls Jan 7, 2019
@TYPO3 TYPO3 deleted a comment from coveralls Jan 7, 2019
@TYPO3 TYPO3 deleted a comment from coveralls Jan 7, 2019
Enables assignment of template variables with a dotted
path as name:

    $view->assign(‘parent.property’, ‘newValue’);

Each value in the path leading up to the final property
name is created as an array if it does not exist, and is
read by reference until a subject is identified. Then the
value is set on that subject.

If a path points to an object and the path to the property
contains one or more scalar values, assignment is
refused with an exception clearly stating why a subject
could not be resolved.

The feature is also enabled for the JsonVariableProvider
by making it call the StandardVariableProvider’s method.
@NamelessCoder NamelessCoder force-pushed the feature/assign-and-override-with-dotted-path branch from a2d4b14 to fe6d213 Compare January 7, 2019 17:37
Copy link
Contributor

@mbrodala mbrodala left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By now we should consider moving this logic to a separate helper class. This would reduce the size of the provider.

@NamelessCoder
Copy link
Contributor Author

By now we should consider moving this logic to a separate helper class. This would reduce the size of the provider.

It used to be like that (see VariableExtractor) but this was refactored a while ago because you can't override VariableExtractor. I would prefer to keep this in a single class because the other way was kind of a mistake.

@yol
Copy link

yol commented Aug 6, 2020

@mbrodala What do you think? Can it be merged as-is? Sitting for some time now :)

@mbrodala
Copy link
Contributor

mbrodala commented Aug 6, 2020

To be honest I think this is out of scope for Fluid. Constructing data should be done before assigning it to the view. And the suggested behavior of modifying existing subjects would mean that variable assignment to a view can have unexpected side effects: if the subject is an object, its state will be different after the assignment to the view, which doesn't make sense.

So from my POV I'd not merge this.

@NamelessCoder
Copy link
Contributor Author

@mbrodala I'd like to discuss this with the TYPO3 core team first; one of the major drawbacks that this patch aims to solve, is the inability to create structured variables through the "data processor" logic used in TYPO3. I do tend to agree with you that this is "before Fluid" concerns, but there may be a reasonable use case to argue for!

@mbrodala
Copy link
Contributor

mbrodala commented Aug 6, 2020

@NamelessCoder Sure thing, please keep me posted.

@yol
Copy link

yol commented Aug 6, 2020

My use case is basically this: I have a gallery partial that has many and hierarchically structured options passed via a settings variable. In a particular template, I want to use most of the settings as they are already defined (via TypoScript, flexform etc.), but modify/override some particular ones. Right now, my only option is to reconstruct the whole data structure with the original values and replace the ones I want to. This is terrible to maintain. My hope was to be able to do something like this: <f:variable name="gallerySettings.colsPerRow.xs" value="1" />

@NamelessCoder Are you also thinking along those lines or do you have further-reaching goals?

@NamelessCoder
Copy link
Contributor Author

@NamelessCoder Are you also thinking along those lines or do you have further-reaching goals?

No, this is pretty much the use case that the patch aims to solve - the place where you do those overrides is arbitrary and f:variable would also support it through this feature patch. The only related thing I've thought about doing is to allow array union {arrayA + arrayB} which would fit your use case, but wouldn't fit the "assign from TS" one.

@yol
Copy link

yol commented Mar 15, 2021

@NamelessCoder did you have the time to discuss this with the TYPO3 core team?

@NamelessCoder
Copy link
Contributor Author

@yol The patch is likely not acceptable in its current form due to the ability to mutate objects through setter methods. I'd still say it is problem-free to assign things in arrays or ArrayObject/ArrayAccess implementations, but the setter method calling would have to be removed.

@yol
Copy link

yol commented Mar 25, 2021

@NamelessCoder That would be fine by me, I guess. It would still cover my use case :) And I can see how calling setter methods from fluid in this generic fashion could lead to unexpected effects.

@spthiel
Copy link

spthiel commented Dec 12, 2022

As I noted in #671 another usecase of this would be to construct multiple arguments of f:render into a single argument to easen the use of a partial (the example I gave was having min, max and additionalArguments on an f:render and then combining them into a single dictrionary for f:form.textfield#additionalArguments).

This is a usecase where the data cannot be constructed outside of fluid and for that the argument given at the top would not apply

@q-u-o-s-a
Copy link

My use case is basically this: I have a gallery partial that has many and hierarchically structured options passed via a settings variable. In a particular template, I want to use most of the settings as they are already defined (via TypoScript, flexform etc.), but modify/override some particular ones. Right now, my only option is to reconstruct the whole data structure with the original values and replace the ones I want to. This is terrible to maintain. My hope was to be able to do something like this: <f:variable name="gallerySettings.colsPerRow.xs" value="1" />

@NamelessCoder Are you also thinking along those lines or do you have further-reaching goals?

It seams for me that you need a kind of config service that return final config for your partial

@lolli42
Copy link
Member

lolli42 commented May 4, 2023

This is related to #35 which I just closed.

This PR increases complexity quite heavily on basically the most-often used runtime object, and thus has quite some negative impact on performance.

I think we should instead reduce complexity further - even more than I recently did with latest StandardVariableProvider changes already - and should prioritize performance over having a more sophisticated variable string parser: There is still quite some potential for performance when pre-optimizing lookups in compliled templates, which we should deal with, first.

We may look at quoting dots and curly braces after that again.

As such, I'll close this PR for now.

@lolli42 lolli42 closed this May 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants