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 Role to be used for use cases like defining dates #143

Closed
gmeligio opened this issue Dec 9, 2020 · 10 comments · Fixed by #164
Closed

Allow Role to be used for use cases like defining dates #143

gmeligio opened this issue Dec 9, 2020 · 10 comments · Fixed by #164

Comments

@gmeligio
Copy link

gmeligio commented Dec 9, 2020

My use case is similar to the ones provided in the Example 2 for hasOccupation and in this use case for worksFor.

As stated in Schema.org hasOccupation definition: The Person's occupation. For past professions, use Role for expressing dates. In the case of worksFor is not stated explicitly but what I want is to represent additional information to a Person, as the original blog post suggests.

I'm working on a PR to solve this but in the meantime I wanted to see if the idea is right for this project.

Here is an example taken from the Role webpage:

{
    "@context": "https://schema.org",
    "@type": "Person",
    "name": "Delia Derbyshire",
    "sameAs": "http://en.wikipedia.org/wiki/Delia_Derbyshire",
    "alumniOf": {
        "@type": "OrganizationRole",
        "alumniOf": {
            "@type": "CollegeOrUniversity",
            "name": "University of Cambridge",
            "sameAs": "http://en.wikipedia.org/wiki/University_of_Cambridge"
        },
        "startDate": "1959"
    }
}
@Eyas
Copy link
Collaborator

Eyas commented Dec 10, 2020

Interesting.

alumniOf is expected to be of type Organization. OrganizationRole is not an Organization. But schema.org shows examples of OrganizationRole being used in its place.

I think the right answer is to open a bug against schema.org. Let's keep this open though, I'll link it

@gmeligio
Copy link
Author

You are right. I didn't notice that the schema was being fetched from https://schema.org/version/latest/schemaorg-current-https.nt and then processed by schema-dts

I was implementing a PR based on the generated schema.d.ts. Didn't check the source code beforehand. I will pause the work on that PR.

Thank you for opening the issue in the right repo. I will follow up there and come back here if I have new reasons.

@Eyas
Copy link
Collaborator

Eyas commented Dec 12, 2020

Schema.org conformance already allows "Text" and "URL" on more fields than what their schema says. Apparently, technically, Role is also allowed on most properties.

I'll see what Richard says in the upstream bug, but we might additionally end up having something like wellKnownStrings for Role properties that are "commonly used" but not described by schema.

@gmeligio
Copy link
Author

gmeligio commented Dec 15, 2020

With that approach, most cases will be covered.

Maybe Role can be allowed in all types, as the conformance section says.

One property of Role is that it has exactly a child node with the same name as the Role's parent. With that in mind, here is the idea I was pursuing. I don't know yet how to make it possible in the generator code, but this is how it would look in the generated schema.d.ts. Surely it will have to be adapted to what would be defined in upstream. Using Record and enforcing the string type will effectively allow restricting to only a child node with the same name as the Role's parent.

declare type RoleValueBase<R extends string, T> = Role & Record<R, T>;
declare type RoleValueLeaf<R extends string, T> = T | RoleValueBase<R, T>;
declare type SchemaRoleValue<R extends string, T> = RoleValueLeaf<R, T> | RoleValueLeaf<R, T>[];
declare type SchemaValue<T> = T | readonly T[];

"hasOccupation"?: SchemaRoleValue<"hasOccupation", Occupation | IdReference>;

@Eyas
Copy link
Collaborator

Eyas commented Dec 15, 2020

Yeah. I assume we could do something simpler, closer to:

"hasOccupation"?: SchemaValue<Occupation | IdReference | Role<"hasOccupation", Occupation>>;

or similar.

I'd still be worried about it for a few reasons:

  • It explodes the size of the schema quite significantly
  • We're already dealing with the fallout of Schema.org being so complicated, e.g. JavaScript heap out of memory #34
  • It gets confusing given that specific types like OrganizationRole exist. I.e., in most cases we want to nudge you to use types like OrganizationRole in the workFor/alumniOf example.

What is certainly true is that:

  1. When Role is in the schema, we need to generate very specific TS code
  2. Properties that contain a Role (maybe all properties, we'll have to decide) need to do a very specific thing

But you're right, even when the Schema.org Issue is addressed for the types you care about, we'll still need a way for role to support these excess properties very specifically.

@shmuelie
Copy link

@Eyas Any thoughts on how I can patch this. Not looking for a permanent solution but like @gmeligio I want to use Role to list past professions and I'd like to keep using type checking of my objects.

@Eyas
Copy link
Collaborator

Eyas commented Dec 29, 2020

@SamuelEnglard if it's okay to skip type checking for just the Role sub-object, I'd probably just cast the role:

({
  "@type": "Role",
  // ...
} as {} as Organization)

or similar, to the desired type.

Eyas added a commit to Eyas/schema-dts that referenced this issue Aug 7, 2021
@Eyas
Copy link
Collaborator

Eyas commented Aug 7, 2021

Hi @shmuelie @gmeligio I have PR #164 which attempts to fix this.

This is a pretty risky change, I published schema-dts@0.11.0-beta.1 as an example.

Can you install this and see if it meets your needs?

@Eyas Eyas closed this as completed in #164 Aug 17, 2021
@gmeligio
Copy link
Author

Hi @Eyas. I've been testing it today, and it has worked great so far.

I hope it can be released as stable in the future.

Thank you very much for the excellent work.

@Eyas
Copy link
Collaborator

Eyas commented Aug 17, 2021

fyi just published 0.11.0, still working on GitHub release notes.

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.

3 participants