Skip to content

FEATURE: Fusion objectPropTypes:FromPrototype to use propTypes from other fusion prototypes#20

Merged
mficzel merged 15 commits intomasterfrom
feature/propsForPrototypes
Jan 9, 2023
Merged

FEATURE: Fusion objectPropTypes:FromPrototype to use propTypes from other fusion prototypes#20
mficzel merged 15 commits intomasterfrom
feature/propsForPrototypes

Conversation

@mficzel
Copy link
Member

@mficzel mficzel commented Nov 11, 2022

The prototype PackageFactory.AtomicFusion.PropTypes:ForPrototype allows to define a DataStructureValidator for all props of the prototype specified via prototypeName.

   @propTypes {
       button = PropTypes:FromPrototype {
          prototypeName = 'Vendor.Site:Button'
       }
   }

Resolves: #16

In addition prototypes are added that allow to specify validation in fusion instead of eel

@propTypes {
    # all props can be marked as required via `@required = true`
    int = PropTypes:Int {
      @required = true
    }
    float = PropTypes:Float
    bool = PropTypes:Bool

    # strings allow to specify an optional `regularExpression`
    string = PropTypes:String {
        regularExpression = '/hello world/'
    }

    # allows array values that satisfy the `type`
    array = PropTypes:Array {
        type = PropTypes:Int
    }

    # allow values that satisfy one of the given validators
    union = PropTypes:Union {
        int = PropTypes:Int
        string = PropTypes:String
        ...
    }

    # allow exacly the values that are specified
    enum = PropTypes:Enum {     
      value1 = "foo"
      value2 = "bar"
      ...
    }

    # a nested structure that is valid once all children valídate
    dataStructure = PropTypes:DataStructure {
        title = PropTypes:String
        description = PropTypes:String
        ...
    }

    # a php object that satisfies the given interface
    instanceOf = PropTypes:InstanceOf {
        type = '\DateTimeInterface'
    }

    # data structure validator that uses the defined proptypes from another prototype
    fromPrototype = PropTypes:FromPrototype {
        prototypeName = "Vendor.Site:Prototype"
    }
}

Additionally:

…ther prototypes

The prototype `PackageFactory.AtomicFusion.PropTypes:ForPrototype` allows to define a DataStructureValidator for all props
of the prototype specified via `prototypeName`.

```
   @propTypes {
       button = PackageFactory.AtomicFusion.PropTypes:ForPrototype {
          prototypeName = 'Vendor.Site:Button'
       }
   }
```

Additionally:
- seperate shape validators are removed ... shape is now an alias for datastructure
- fusion-prototypes for Bool, Float, Int, String, DataStructure, ArrayOf, AnyOf, OneOf are added
@mficzel mficzel force-pushed the feature/propsForPrototypes branch from d042c4b to f46b05e Compare November 11, 2022 13:40
@mficzel mficzel changed the title WIP: Create proptypes for other fusion prototypes WIP: Create ForPrototype protype to use propTypes from other fusion prototypes Nov 11, 2022
@mficzel mficzel requested a review from grebaldi November 11, 2022 13:59
@mhsdesign
Copy link
Contributor

hi öhm #17 hmmm

@mficzel
Copy link
Member Author

mficzel commented Nov 11, 2022

@mhsdesign did not see that, sorry ... however this pr allows to specify all proptypes as fusion object

@mficzel mficzel changed the title WIP: Create ForPrototype protype to use propTypes from other fusion prototypes FEATURE: Create ForPrototype protype to use propTypes from other fusion prototypes Nov 11, 2022
@mficzel mficzel marked this pull request as ready for review November 11, 2022 15:09

class AnyOfImplementation extends AbstractArrayFusionObject
{
use AbstractValidatorTrait;
Copy link
Contributor

Choose a reason for hiding this comment

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

traits make me happy :D


public function createValidator(): DataStructureValidator
{
$prototypeName = $this->fusionValue('prototypeName');
Copy link
Contributor

Choose a reason for hiding this comment

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

i would argue that "prototype" is the wrong name, its too unspecific propTypes can only be used with components ... so how about ForComponent and componentName ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Used it as it is the same as in Sitegeist.Monocle:PreviewPrototype ... bettter suggestions are welcome. Also it would be great to specify a prototype not as string. Did not find a good way to achieve this.

Copy link
Contributor

Choose a reason for hiding this comment

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

i now some dark magic so you can do

type = Foo.Bar
(without strings ^^)

but thats not really tge fusion way, i think people got used to it ...

Copy link
Member Author

@mficzel mficzel Nov 11, 2022

Choose a reason for hiding this comment

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

Und dann runtime->evaluate->("type/__meta/propTypes"); ... hmmm really dark. If we find a way to achieve this in a way that works for Sitegeist.Monocle:PreviewPrototype i would love to see this.

Copy link
Contributor

Choose a reason for hiding this comment

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

hmm one can access the object name via $this->properties["type"]['__objectType'] ;)

(tested)

Copy link
Member Author

Choose a reason for hiding this comment

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

@mhsdesign Would still like to go with "string:prototypeName" first. If we find ab better way we can implement the same solution in monocle and here.

@mficzel
Copy link
Member Author

mficzel commented Nov 11, 2022

@grebaldi i wonder how this could be adapted to Sitegeist.Monocle.PropTypes ... pretty sure there is a way but i do not see it yet.

@mficzel
Copy link
Member Author

mficzel commented Nov 14, 2022

this is how short aliases could look:

prototype(PropTypes:Int) < prototype(PackageFactory.AtomicFusion.PropTypes:Int) {}
prototype(PropTypes:Float) < prototype(PackageFactory.AtomicFusion.PropTypes:Float) {}
prototype(PropTypes:Bool) < prototype(PackageFactory.AtomicFusion.PropTypes:Bool) {}
prototype(PropTypes:String) < prototype(PackageFactory.AtomicFusion.PropTypes:String) {}
prototype(PropTypes:ArrayOf) < prototype(PackageFactory.AtomicFusion.PropTypes:ArrayOf) {}
prototype(PropTypes:AnyOf) < prototype(PackageFactory.AtomicFusion.PropTypes:AnyOf) {}
prototype(PropTypes:OneOf) < prototype(PackageFactory.AtomicFusion.PropTypes:OneOf) {}
prototype(PropTypes:DataStructure) < prototype(PackageFactory.AtomicFusion.PropTypes:DataStructure) {}
prototype(PropTypes:InstanceOf) < prototype(PackageFactory.AtomicFusion.PropTypes:InstanceOf) {}
prototype(PropTypes:For) < prototype(PackageFactory.AtomicFusion.PropTypes:ForPrototype) {}

What i would still like to change / discuss:

  • Use a better name for "OneOf" to avoid confusion with "AnyOf" ... maybe PropTypes:Enum
  • Maybe use PropTypes:DataStructure.forPrototype instead of PropTypes:For ...
  • Maybe use PropTypes:Object.type instead of PropTypes:InstanceOf

@mficzel
Copy link
Member Author

mficzel commented Nov 14, 2022

IDEA: combine OneOf and AnyOf

PropTypes:OneOf {
	0 = PropTypes:Int
	1 = "foo"
	2 = "bar"
}
´

PropTypes:ArrayOf {
	0 = PropTypes:Int
	1 = "foo"
	2 = "bar"
}

PropTypes:DataStructure{
	number  = PropTypes:Int
	text = PropTypes:String
}

... also added short aliases for the propTypes
@mficzel mficzel changed the title FEATURE: Create ForPrototype protype to use propTypes from other fusion prototypes FEATURE: Create PropTypes:ForPrototype protype to use propTypes from other fusion prototypes Nov 15, 2022
@mficzel mficzel changed the title FEATURE: Create PropTypes:ForPrototype protype to use propTypes from other fusion prototypes FEATURE: Fusion objectPropTypes:ForPrototype to use propTypes from other fusion prototypes Nov 15, 2022
@mficzel mficzel force-pushed the feature/propsForPrototypes branch from 36952cf to 741b7cc Compare November 15, 2022 12:46
Copy link
Member

@grebaldi grebaldi left a comment

Choose a reason for hiding this comment

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

Hi @mficzel,

thanks for building this 👍 , I like the idea.

I added some conceptual comments, but mostly typo stuff :)

RE @mficzel:

@grebaldi i wonder how this could be adapted to Sitegeist.Monocle.PropTypes ... pretty sure there is a way but i do not see it yet.

Oh, I'd be very pleased to take a look at that :)

RE @mficzel:

IDEA: combine OneOf and AnyOf

+1 - And to reduce confusion even more, I'd like to suggest to drop the react-proptypes API naming in this case and call the combined one Union:

myValue = PropsTypes:Union {
    0 = PropTypes:Int
    1 = PropTypes:Float
}

@mhsdesign
Copy link
Contributor

i mus say i also really like PropsTypes:Union ;)

@mficzel mficzel force-pushed the feature/propsForPrototypes branch from 2e99613 to 613ace3 Compare December 23, 2022 09:58
@mficzel
Copy link
Member Author

mficzel commented Dec 23, 2022

I added PropTypes:Union and PropTypes:Enum as those are distinct cases. Other than that i think it might actually be ready now.

I kept "prototypeName" for now as it is in line with what Monocle:PreviewPrototype does. Once we have a better solution there i would like to add a "type" that accepts a FusionPrototype without Quotes in another PR.

@mficzel mficzel changed the title FEATURE: Fusion objectPropTypes:ForPrototype to use propTypes from other fusion prototypes FEATURE: Fusion objectPropTypes:FromPrototype to use propTypes from other fusion prototypes Dec 23, 2022
mficzel added a commit to sitegeist/Sitegeist.Monocle.PropTypes that referenced this pull request Dec 23, 2022
…facade

This ensures that the solution is compatible with the upcoming changes in
proptypes PackageFactory/atomic-fusion-proptypes#20
@mficzel
Copy link
Member Author

mficzel commented Dec 23, 2022

@grebaldi prepared the adjustments for monocle-proptypes. Seems to work with buth styles of proptypes ... do you consider this approach valid
sitegeist/Sitegeist.Monocle.PropTypes#3

mficzel added a commit to sitegeist/Sitegeist.Monocle.PropTypes that referenced this pull request Dec 23, 2022
…facade

This ensures that the solution is compatible with the upcoming changes in
proptypes PackageFactory/atomic-fusion-proptypes#20
mficzel added a commit to sitegeist/Sitegeist.Monocle.PropTypes that referenced this pull request Dec 23, 2022
…facade

This ensures that the solution is compatible with the upcoming changes in
proptypes PackageFactory/atomic-fusion-proptypes#20
mficzel added a commit to sitegeist/Sitegeist.Monocle.PropTypes that referenced this pull request Dec 23, 2022
…facade

This ensures that the solution is compatible with the upcoming changes in
proptypes PackageFactory/atomic-fusion-proptypes#20
@grebaldi grebaldi self-requested a review January 6, 2023 19:29
@mficzel mficzel merged commit 7b9e475 into master Jan 9, 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.

Remove Neos.Fusion:RawArray from dynamic @propTypes path evaluation Allow to reference inside PropTypes another Fusion components api

3 participants