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

Person does not inherit correctly from interfaces PersonLeaf -> PersonBase #170

Closed
TJKoury opened this issue Feb 22, 2022 · 10 comments
Closed

Comments

@TJKoury
Copy link

TJKoury commented Feb 22, 2022

Not sure if this is a Typescript, bug or a schema issue, but here goes:

Importing:

import type { Person } from "schema-dts";

Throws the TypeScript error: Property 'familyName' does not exist on type 'Person'. Property 'familyName' does not exist on type 'string'.;

Clearly it is ignoring the logical 'or' in export declare type Person = PersonLeaf | Patient | string; and skipping directly to 'string'. Not sure why.

@TJKoury TJKoury changed the title Person does not inherit correctly from interfaces PersonLeaf -> PersonBase in VSCode Person does not inherit correctly from interfaces PersonLeaf -> PersonBase Feb 22, 2022
@Eyas
Copy link
Collaborator

Eyas commented Feb 23, 2022

This could be an issue with narrowing that results in a bad error message.

In general, if you don't yet have a "@type": "Person" (or "Patient") on your object that's defined as a Person, you'll get confusing errors. Can you show me a sample of the code you have and exactly where the error is?

@TJKoury
Copy link
Author

TJKoury commented Feb 23, 2022

Check it out here.

NAME="Pop!_OS"
VERSION="21.10"
ID=pop
ID_LIKE="ubuntu debian"
PRETTY_NAME="Pop!_OS 21.10"
VERSION_ID="21.10"
HOME_URL="https://pop.system76.com"
SUPPORT_URL="https://support.system76.com"
BUG_REPORT_URL="https://github.com/pop-os/pop/issues"
PRIVACY_POLICY_URL="https://system76.com/privacy"
VERSION_CODENAME=impish
UBUNTU_CODENAME=impish
LOGO=distributor-logo-pop-os
Node v16.13.1

This is the error I'm getting:

index.ts:4:12 - error TS2339: Property 'additionalName' does not exist on type 'Person'.
  Property 'additionalName' does not exist on type 'string'.

4     person.additionalName = "POD";

@TJKoury
Copy link
Author

TJKoury commented Feb 23, 2022

I referenced a solution in this pull request. I closed the pull and just published my own fork to npm because I have a deadline, but definitely open to working the issue with you.

@Eyas
Copy link
Collaborator

Eyas commented Feb 23, 2022

DigitalArsenal/schema-dts-test is a private repo, do you mind sharing access?

@TJKoury
Copy link
Author

TJKoury commented Feb 23, 2022

Wow, sorry about that, I literally created it just for this thread.

@Eyas
Copy link
Collaborator

Eyas commented Feb 23, 2022

Got it. Yeah, this is indeed invalid:

const podifyPerson = (person: Person) => {
    person.additionalName = "POD";
}

because a Person can absolutely be a string.

If you expect Person not to be a string, you can use Exclude<Person, string>. Which is equivalent to PersonLeaf | PatientLeaf.

There is a request somewhere to export XLeaf properties directly. My concern is that it will encourage improper use (i.e. people using PersonLeaf when they are okay with any subtype of Person).

@TJKoury
Copy link
Author

TJKoury commented Feb 23, 2022

That's interesting. I'm not sure in what cases Person could be a string, especially given the Schema.org definition. Can you explain where that came from?

@TJKoury
Copy link
Author

TJKoury commented Feb 23, 2022

I also suppose that one could sub-type it like so to reduce verbosity, at the same time that only makes sense if you are going to extend / modify it in some way:

import { Person } from "schema-dts";
import { SLIP_0044_TYPE } from "./slip_0044";

export type cryptoKeyBase = {
    //hex publicKey
    publicKey: string,
    privateKey?: string,
    keyAddress?: string,
    //https://github.com/satoshilabs/slips/blob/master/slip-0044.md
    keyType?: SLIP_0044_TYPE | number
}

export interface cryptoKey extends cryptoKeyBase {
    "@type": "CryptoKey",
}

export type PersonPublicKey = Exclude<Person, string> & {
    key: cryptoKey | Array<cryptoKey>
}

@Eyas
Copy link
Collaborator

Eyas commented Feb 23, 2022

I'm not sure in what cases Person could be a string, especially given the Schema.org definition. Can you explain where that came from?

See #37 for more details (and #19), but technically in Schema.org, all objects can be referred to as strings. But only a small subset is frequently used.

This is typically seen in nested properties, e.g. "author" and "contentLocation" on, say, an ImageObject, where a stirng is used for a Person/Place. See https://schema.org/ImageObject#eg-0021 for example

For the purposes of this library, instead of allowing strings on all objects, we opt to only allow strings on objects where cannonical Schema.org examples show them used as strings.

@TJKoury
Copy link
Author

TJKoury commented Feb 23, 2022

I see it. Seems more like an oversight to me, since the Person example uses Jane Doe as the name property, making Person (and others) technically recursive data structures, as the name would be presumably referencing the same entity as the parent Person object.

That is a different topic though, I understand why you made that design decision. Thanks for the explanation.

@TJKoury TJKoury closed this as completed Feb 23, 2022
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

No branches or pull requests

2 participants