Skip to content
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

Allow more primitives (boolean/number/string) #37

Closed
2 of 3 tasks
OliverJAsh opened this issue Oct 9, 2019 · 8 comments · Fixed by #47
Closed
2 of 3 tasks

Allow more primitives (boolean/number/string) #37

OliverJAsh opened this issue Oct 9, 2019 · 8 comments · Fixed by #47

Comments

@OliverJAsh
Copy link

OliverJAsh commented Oct 9, 2019

E.g. in ImageObject:

  • isAccessibleForFree should be allowed to be a boolean primitive
  • width and height should be allowed to be a number primitive
  • author and contentLocation should be allowed to be a string primitive

Related: #19

@OliverJAsh OliverJAsh changed the title Allow boolean primitives for Boolean type Allow boolean/number primitives Oct 9, 2019
@OliverJAsh OliverJAsh changed the title Allow boolean/number primitives Allow more primitives (boolean/number/string) Oct 9, 2019
@Eyas
Copy link
Collaborator

Eyas commented Oct 9, 2019

Thanks for the suggestions. This is helpful. For each of these, I'm basically curious if these semantics are seen as "canonical" in general. Any data / leads here will be helpful:

For Boolean types, my understanding is that these are expected to be assignable to "https://schema.org/True" and "https://schema.org/False" (from looking at Schema.org examples using Boolean). Do you have an example of true/false JSON primitives being accepted?

For width/height -- these are type Distance of QuantitativeValue. My understanding of both those types is that they're meant to encode a value with some unit attached to it. #19 as you mentioned supported using "string" for these values. I.e. "12 m" (or "13px"). Seems contrary to the intent of the schema to make distance potentially unit-less?

Ack on author/contentLocation. I know Person and Place are both accepted the Google Structured Data Testing Tool as stand-ins for {"@type": ..., "name": <string value> }. My hope was to be slightly opinionated here and allow the equivalent but more verbose version. Does this slow people down in the wild? Curious about your use cases.

@OliverJAsh
Copy link
Author

Do you have an example of true/false JSON primitives being accepted?

See the first example here: https://schema.org/isAccessibleForFree

image

Seems contrary to the intent of the schema to make distance potentially unit-less?

Good point, maybe ignore my suggestion that number should be allowed!

My hope was to be slightly opinionated here and allow the equivalent but more verbose version. Does this slow people down in the wild? Curious about your use cases.

That makes sense. I'm fine with that.

Another one to consider: allow thumbnail to be a string (URL). Or would you group that with author/contentLocation, i.e. require an object?

@OliverJAsh
Copy link
Author

My hope was to be slightly opinionated here and allow the equivalent but more verbose version. Does this slow people down in the wild? Curious about your use cases.

On second thoughts, it seems a shame if the types don't work with the examples provided on https://schema.org/, e.g. https://schema.org/ImageObject uses string for author and contentLocation.

image

@Eyas
Copy link
Collaborator

Eyas commented Oct 25, 2019

For Booleans, to save backwards compatibility, a good shape would be:

enum BooleanEnum {
    True = "https://schema.org/True",
    False = "https://schema.org/False"
}
/** Boolean: True or False. */
export type Boolean = true | false | BooleanEnum;
export const Boolean = {
  True: BooleanEnum.True as const,
  False: BooleanEnum.False as const,
}

So you can use:

  • Boolean.True,
  • Boolean.False,
    as well as true and false.

@Eyas
Copy link
Collaborator

Eyas commented Oct 25, 2019

Ended up with something slightly different:

/** Boolean: True or False. */
export declare type Boolean = true | false | "https://schema.org/True" | "https://schema.org/False";
export declare const Boolean: {
    True: "https://schema.org/True" as const;
    False: "https://schema.org/False" as const;
};

This also allows users to assign the strings as-is, in addition to Boolean.True and Boolean.False.

@Eyas
Copy link
Collaborator

Eyas commented Nov 12, 2019

#47 will fix most of this. Note that, for

Another one to consider: allow thumbnail to be a string (URL). Or would you group that with author/contentLocation, i.e. require an object?

won't be fixed. As far as I can tell, SDTT complains if ImageObject is declared a string. Let me know if I'm wrong here.

@Eyas Eyas closed this as completed in #47 Nov 12, 2019
@OliverJAsh
Copy link
Author

@Eyas Thanks for doing that!

As for thumbnail, here's an example on the official schema website demonstrating thumbnail as a string:

image

https://schema.org/VideoObject

@Eyas
Copy link
Collaborator

Eyas commented Nov 13, 2019

Hm. Note that ImageObject doesn't always work as a string, for example:

https://lists.w3.org/Archives/Public/public-schemaorg/2019Nov/0003.html

Structured Data Testing Tool rejects when "logo" is set to a string, requiring an ImageObject set explicitly.

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 a pull request may close this issue.

2 participants