fluid-components icon indicating copy to clipboard operation
fluid-components copied to clipboard

Possible legal values for parameters

Open ulrichmathes opened this issue 2 years ago • 7 comments

It would be nice to define possible legal values for an parameter. Without having to introduce a custom data structure.

<fc:param name="align" type="string" cases="{'left', 'right'}" />

// or more slimline with cases fromString:
<fc:param name="align" type="string" cases="left,right" />
// or ?
<fc:param name="align" type="string" cases="left|right" />

I think an implementation would not a big deal.

I thought about to enable enum as parameter type but we would not be able to pass an enum case in an fluid template. So the value of the parameter would be a string anyway. To then pass the string to the tryFrom function of the enum does not feel right and overdone. Maybe enum will have usecases but for many cases I can think of now, a simple list, defined within the parameter tag would be very handy.

[written on smartphone, pull request will follow]

ulrichmathes avatar Sep 19 '23 22:09 ulrichmathes

Does such a feature makes sense?

ulrichmathes avatar Sep 20 '23 07:09 ulrichmathes

IMO it does but it can't be implemented like this as far as I could see. The parameters are converted to native fluid parameters and there it is not possible to add an additional attribute like yours. Our workaround was to implement an additional viewhelper that does take care of value validation. Enum would also be something that should be supported but this does also not solve many uses cases (like if something should be in a range of numbers or something like that). I think adding validators like in extbase would be the best way to solve this but i think this would require a bigger change due to the reason mentioned in the beginning.

sascha-egerer avatar Sep 20 '23 07:09 sascha-egerer

This is the viewhelper I've used for such cases


<?php
declare(strict_types = 1);

namespace xxx\xxx\ViewHelper;

use SMS\FluidComponents\Domain\Model\ComponentInfo;
use SMS\FluidComponents\Exception\InvalidArgumentException;
use TYPO3Fluid\Fluid\Core\Rendering\RenderingContextInterface;
use TYPO3Fluid\Fluid\Core\ViewHelper\AbstractViewHelper;
use TYPO3Fluid\Fluid\Core\ViewHelper\ArgumentDefinition;
use TYPO3Fluid\Fluid\Core\ViewHelper\Traits\CompileWithRenderStatic;

class ParamValidatorViewHelper extends AbstractViewHelper
{
    use CompileWithRenderStatic;

    /**
     * @var bool
     */
    protected $escapeOutput = false;

    public function initializeArguments(): void
    {
        $this->registerArgument('name', 'string', 'Name of variable to validate', true);
        $this->registerArgument('allowed', 'array', 'List of values to validate against', true);
        $this->registerArgument('inList', 'bool', 'Validate if the given values are in the given list', false, false);
    }

    /**
     * @param array<mixed> $arguments
     */
    public static function renderStatic(array $arguments, \Closure $renderChildrenClosure, RenderingContextInterface $renderingContext): string
    {
        $propertyValue = $renderingContext->getVariableProvider()->get($arguments['name']);
        $componentInfo = $renderingContext->getVariableProvider()->get('component');
        $allowedValues = $arguments['allowed'];
        $componentName = $renderingContext->getControllerName() . '::' . $renderingContext->getControllerAction();
        if ($componentInfo instanceof ComponentInfo) {
            $componentName = $componentInfo->namespace ?? $componentName;
            $argumentDefinition = $componentInfo->argumentDefinitions[$arguments['name']] ?? null;
            if (!$argumentDefinition instanceof ArgumentDefinition) {
                throw new \LogicException(sprintf('Can not validate values for param "%s", because it is not registered in component "%s"', $arguments['name'], $componentName), 1670863756);
            }
            if (!$argumentDefinition->isRequired()) {
                // If an optional property is not set on the tag, Fluid will assign the default value to the argument
                // Hence we allow default values as allowed values
                // This also means if no default is set (default is null),
                // that null values are allowed to be passed to the tag property (e.g. through a variable)
                $allowedValues[] = $argumentDefinition->getDefaultValue();
                // If a default value is set for a component prop and the property on the tag is set as well,
                // but a null value is explicitly passed (e.g. <s:component.headline type="1" style="{does.not.exist}" />),
                // Fluid Components assigns this null value to the prop instead of the default.
                // To avoid to handle this case with a condition in the templates, we modify the value
                // in the variable provider and set it to the default value gracefully.
                if ($propertyValue === null && $argumentDefinition->getDefaultValue() !== null) {
                    $propertyValue = $argumentDefinition->getDefaultValue();
                    // This is a side effect, which is debatable, I'd argue this is a workaround
                    // for an inconsistent behaviour in Fluid/ Fluid Components
                    $renderingContext->getVariableProvider()->add($arguments['name'], $propertyValue);
                }
            }
        }

        $throwException = static function ($value, $allowedValues, string $argumentName, string $componentName): void {
            throw new InvalidArgumentException(sprintf(
                'The given value "%s" for option "%s" is invalid in "%s". Possible values are: %s',
                var_export($value, true),
                $argumentName,
                $componentName,
                implode(
                    ', ',
                    array_map(static fn ($item): string => var_export($item, true), $allowedValues)
                )
            ), 1667555651);
        };

        if (is_array($propertyValue)) {
            if (!self::arrayIsAllowedValue($allowedValues, $propertyValue, $arguments['inList'] ?? false)) {
                $throwException($propertyValue, $allowedValues, $arguments['name'], $componentName);
            }

            return '';
        }

        if (!in_array($propertyValue, $allowedValues, true)) {
            $throwException($propertyValue, $allowedValues, $arguments['name'], $componentName);
        }

        return '';
    }

    /**
     * @param array<mixed> $allowedValues
     * @param array<mixed> $value
     */
    private static function arrayIsAllowedValue(array $allowedValues, array $value, bool $inList = false): bool
    {
        if ($inList) {
            $isAllowed = false;
            foreach ($value as $singleValue) {
                $singleValueResult = self::arrayIsAllowedValue(
                    array_map(fn ($item): array => [$item], $allowedValues),
                    [$singleValue]
                );
                if (!$singleValueResult) {
                    return false;
                }
                $isAllowed = true;
            }
            return $isAllowed;
        }
        foreach ($allowedValues as $allowedValue) {
            if (!is_array($allowedValue)) {
                continue;
            }

            if ($value === $allowedValue) {
                return true;
            }
        }

        return false;
    }
}

sascha-egerer avatar Sep 20 '23 07:09 sascha-egerer

Another possibility just for the "in list" case could be to add literal and union types, like TypeScript has:

https://www.typescriptlang.org/docs/handbook/2/everyday-types.html#literal-types

But I'm not sure if it's worth the trouble implementing this.

s2b avatar Sep 20 '23 07:09 s2b

Something like this?

<fc:param name="align" type="literal" cases="{'left', 'right'}" />
<fc:param name="prime" type="literal" cases="{2, 3, 5, 7}" />

// not valid
<fc:param name="prime" type="literal" cases="{'left', 2, 3, 5, 7}" />

ulrichmathes avatar Sep 20 '23 09:09 ulrichmathes

More like:

<fc:param name="align" type="'left' | 'right'" />

s2b avatar Sep 20 '23 11:09 s2b

+1 for this feature!

I found this example for a Button component with three variants. If both params are false, then it's the default variant.

    <fc:param name="isPrimary" type="boolean" optional="1" default="0" />
    <fc:param name="isSecondary" type="boolean" optional="1" default="0" />

In my view this syntax would be much better:

<fc:param name="variant" type="'default' | 'primary' | 'secondary'" />

fgeierst avatar Oct 02 '23 19:10 fgeierst