Update BaseType to serialize Identifier property to '@id' in order conform to LD-JSON specification

[owljester]

https://schema.org/identifier
https://schema.org/docs/datamodel.html#identifierBg

All schema.org syntaxes already have built-in representation for URIs and URLs, e.g. ... in JSON-LD, '@id'. Generally it is preferable to use these unless there is a specific requirement to explicitly state the kind of identifier...

This solution tries to address the concerns raised by PR #72 and Issue #59 in a clean way that will not interfere the Schema generation nor require the user to "hack" in the property via setProperty. It does this by extending the toArray to serialize the "identifier" property as "@id" and remove "identifier".

I believe this library only has the toScript method to print out the schema, so I am making an assumption here that it is safe to make this modification in the toArray method. However, as Schema.org mentions, there are other formats that also have their own URI syntax and each format's syntax should be respected.

[Gummibeer]

Hey,

first of all thanks for your work! I have multiple points so I will try to structure them:

1. Can you please fix the code style? See styleCI
2. The PR contains a lot of changes that aren't related. Could you please revert them? I know that they are from the generator but they should be done in another PR/release.
3. I'm really not this deep into schema.org, RFC and so on. Would the identifier hurt if it is present alongside the @id? I'm just not this happy, also for unittests, if I set identifier and get @id.

[owljester]

Hello,

1&2 - Understood, I'll make those changes

3. I wouldn't call myself an expert, but while researching the identifier vs @id I found that they're essentially the same thing. Schema.org has Identifier property to represent a URI but they appear to expect for the various data formats to output the identifier property as the format's URI. In LD-JSON's case, that would be @id. However - I have the same concern you do. This modification is opinionated - someone who is using identifier and expecting the "identifier" property after using toArray would have a bad time after this patch.

One thought I had would be to create a toLdJson that would be a mirror of the toScript except to include this functionality. This would duplicate the toArray method as well, but re-use the serializeProperty method - while allowing the existing toScript to continue to behave as expected. Users who want the @id rather then identifier property could just use the toLdJson method. (And, it does sort of set a precedent for how to handle outputting other formats should anyone wish to do that).

Let me know if adding toLdJson method, or as you suggested simply not unsetting the identifier property, is the preferred solution on your end & I'll make the required adjustments.

Also, in the collaboration document it mentions to document any changes. Would this warrant a mention on the README? I'm also assuming the changelog is automatically updated or otherwise handled during the release process?

Thanks!

[Gummibeer]

Thanks. 😊

I be honest and would like to request a decision by @sebastiandedeyne .

I definitely would feel better to handle it optional. Primary to prevent a BC release and I'm not an expert.^^
To don't introduce and duplicate too much new for myself I would prefer a chainable method like useJsonLdId() or something like this to set a bool which is used during serialization. I would also like an enable/disable functionality like enableJsonLdId() and disableJsonLdId().
We could also discuss if this setting should be static.

But before you change anything I hope that Sebastian would have time.

And yes, the readme should be adjusted in the PR and the changelog is done by us during release.

[owljester]

I like your idea, having a flag like that and the checking for it in toArray would likely make the least dramatic changes and keep this backwards compatible. I will await input from @sebastiandedeyne .

[sebastiandedeyne]

I think the approach of this PR makes sense.

Is a flag necessary? What are the odds of someone already using identifier for something else?

[Gummibeer]

The problem is that the @id isn't the real schema.org standard but used by most readers. So this is a fundamental decision if this package should go with the real schema.org standard (described in RDFA and read from there) or if we remove parts of it in favor of most common readers?
That's why I would like to hide this behind a flag. This would prevent a BC and also keeps the real schema.org standard as default but allows to switch to something else.

[owljester]

On one hand, this plugin's "toScript" method outputs LD-JSON - so making this change would play well with the handy auto-generation of templates based on the Schema.org documentation while also adhering with Schema.org's own stance to use the identifier property as the "id" for whichever format you're rendering the data. In LD-JSON's case, that is @id. Reference

On the other, by doing this in the current proposal, it would effectively remove "identifier" as a property from the output and that could cause grief to users if they are parsing their output for that property.

I imagine though, that my particular usecase is like many others: making Google's schema parser happy. So they probably were using the fix suggested by Issue #59.

I await a judgement call from the maintainers. The flag is potentially counter-intuitive to new users but preserves backwards compatibility.

I appreciate y'alls input on this!

[Gummibeer]

@practical42 I'm super happy that you shared this link! I have to get deeper into the differences and standards.

My final suggestion would be:

• Add toJsonLd() method
• Flag toScript() as deprecated
• Add transformer like methods as base of the toXyz() methods - they would run before the serializer

At all I would like to have the transforming logic splitter of the serializer and outputter. I also would like to add this non breaking but prepare the next major release. I also would like to prepare other outputters with this, like microdata or whatever.

If this way is accepted and we would do it like this I would request following:
I know that this isn't the cool way and really want to say that others can't do it. So I ask upfront for apologize! But I want to ask if I could do this my own? I will definitely assign @practical42 as author of the @id related commits. But because this is a bigger ToDo with code decisions for upcoming releases and features I would like to have this done by team members and @sebastiandedeyne hasn't much time.
I hope this isn't too bad behavior!? And you understand it.

[owljester]

I am not bothered if you guys would rather handle this internally, its y'alls repo. I would appreciate the author credit where and if appropriate!

[Gummibeer]

👊 @sebastiandedeyne how would we proceed here? Atm the PR is failing. But how do you want to handle this?

[Gummibeer]

Hey @practical42 ,

I'm super sorry for this long delay! The package and it's PR wasn't in my sight the last time and wasn't actively maintained. :(
Because of the conflicts CS errors I've moved your solution, slightly adjusted, including you as contributor in #102 .
After reading all the links and so on and the fact we only support LD+JSON it's kind of a bug-fix. But I'm not sure if we will handle it like one or if the BC nature of this change weights heavier.

Thanks again for your work, time and all the links/quotes you searched for and posted! <3

[Gummibeer]

https://github.com/spatie/schema-org/releases/tag/2.6.0