Type inheritance hints via interfaces to allow type hinting in projects

[graste]

I know, I'm late on the 2.x BC about type inheritance: I e.g. typehinted SchemaOrgEvent on some methods and now that e.g. a SchemaOrgLiteraryEvent is no longer of type SchemaOrgEvent the php typehinting breaks. Good you chose 2.0 release. ;-)

I wonder what your stance is on generating interfaces for the previously subclassed classes so code that is using this lib can typehint IsEvent (or similar) instead of deciding between nothing or BaseType.

I had a look at the old issues but am not sure whether this was an option. I understand there might be problems with the LocalBusiness having (some?) stuff from Organization and Place.

[graste]

Another point: Due to the typehint changes phpstan now throws errors like Call to an undefined method Spatie\SchemaOrg\BaseType::potentialAction() or Parameter #1 $events of method Spatie\SchemaOrg\Place::events() expects array<Spatie\SchemaOrg\Event>|Spatie\SchemaOrg\Event, array<int, Spatie\SchemaOrg\BaseType> given. etc.

[Gummibeer]

Hey,

sorry for the late response. The guys at spatie are super busy and I will try to clean things up now.

I got your point and agree. The reason was that no one, during development, mentioned or thought about any tools like phpstan but only IDE. So we decided to go with @mixin phpdoc annotations and prevent code duplications.
And doing it the real code way would be much harder.
The way I have in mind would tripple the files, which isn't a real reason against but because it has to be done with a dynamic generator (which has to be written) it's much more to concept.

My idea would be to have an interface, trait and class per type.

• The interface would define the required methods.
• The trait would contain all the methods and doc tags.
• The class would only implement the interface and use the trait.

With this we could prevent copy'n'paste the methods into every class (traits) would have type security (interfaces) and allow classes/types to extend multiple types.

Because we already have #66 and #79 in the pipeline combined with a general release (rdfa changed) I would like to wait until they are done.

I'm also open for another, simpler, idea which solves the problem without removing the ability to extend multiple types.

[sebastiandedeyne]

Fwiw, after releasing the @mixin based version I wasn't all to happy about it, because apparently not all editors support it...

My idea would be to have an interface, trait and class per type

I think it'd make sense to revisit this idea at some point.

[Gummibeer]

At all this change wouldn't be breaking. So we could do it at every time (the label is over estimated).
It's just a huge change in the code-base. But the classes the developer uses are the same, they will only get more real methods instead of IDE only ones.

[Gummibeer]

@graste could you check if #97 released as 2.5.0 solves your issue?
We now have interfaces and all methods a type knows about are defined as real method. So the @mixin approach is dropped.

[graste]

Hi, sorry for not replying. Didn't see it. :-)

I like the Contract stuff. Unfortunately I first upgraded to 2.x on a PHP 7.2 project and only got 2.8.0 which has missing EventContract on Event etc. Forcing composer to upgrade only this lib while ignoring php platform requirements made it work after basically changing the code to typehinting SchemaOrgEventContract instead of SchemaOrgEvent. Thus e.g. SchemaOrgLiteraryEvent and generic SchemaOrgEvent can be returned by the same methods even though setProperty or id are not in interfaces, if I see this correctly? Good change though!

Is there a reason for the need for php 7.4? The code seems to run w/ 7.2 which is still supported for 7 months, 7.3 for 1 year and 7 months. A library like this lives on projects that might be conservative to upgrading php versions while still needing updates of the schema org schemas. A bit more conservative approach when new features of newer php versions are not used would be nice. Even though I agree that it's easier for package maintainers and forces people to actually upgrade thei php stacks. :-)

Thanks for your work! May close this issue, I guess. The irregularities of the actual schema org stuff (virtual location etc) are nothing this project can do anything about.

[Gummibeer]

Hey,
we dropped support for all versions that aren't fully supported anymore.
https://www.php.net/supported-versions.php
PHP7.2 is only in security phase and 7.3 is the minimum version.

The reason wasn't really based in the user used code but in the tests and generator code. So yes, we could split dev and prod required PHP versions. But this would again increase the technical depth.
So yeah, it's right that dropping PHP versions always drops some users - but spatie as a whole keeps always to the latest versions and the moment anything requires the drop, even of a still supported version, it's most times dropped. Like you said: possibly this "forces" users to upgrade their own servers.