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

fix(claim): improve SnakValue type #118

Merged
merged 2 commits into from
Jun 10, 2023
Merged

Conversation

EdJoPaTo
Copy link
Contributor

also some minor Claim/Snak typing improvements.

SnakValue typings should be complete based on what I found here.

Something I am not sure about: no Snak (Qualifier and ReferenceSnak included) contains an id (from the real world data I observed.
its optional for Snak but required for ReferenceSnak and Qualifier. I think this is wrong and can be remove on all 3 of them. But I might be missing something so can someone confirm me here? Its there so it probably has a reason?

also some minor Claim/Snak typing improvements
I can't figure out why I put it there in the first place,
otherwise than by confusion, so let's remove it and find out ^^
@maxlath
Copy link
Owner

maxlath commented Jun 10, 2023

Regarding the id? on Snak, I can't recover a valid reason why I put it there (other that by propagating a long gone legacy property, or an old confusion/error, while refactoring/converting to TS), so let's remove it and find out 😅

@maxlath maxlath merged commit 6dabe81 into maxlath:main Jun 10, 2023
@EdJoPaTo EdJoPaTo deleted the snakvalue branch June 13, 2023 11:28
Comment on lines +18 to +21
/** @deprecated use WikibaseItemSnakValue */
export type SnakEntityValue = WikibaseEntityIdSnakValue
/** @deprecated use WikibaseItemSnakValue */
export type ClaimSnakWikibaseItem = WikibaseEntityIdSnakValue
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oups… looks like the deprecated hint points to the wrong object name. Its easy to find out when looking into the code so not that important to fix?

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 this pull request may close these issues.

2 participants