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

Update type definitions for v7 #61

Closed
MrXyfir opened this issue Jun 3, 2019 · 7 comments
Closed

Update type definitions for v7 #61

MrXyfir opened this issue Jun 3, 2019 · 7 comments

Comments

@MrXyfir
Copy link

MrXyfir commented Jun 3, 2019

@maxlath @noinkling @EdJoPaTo

In the meantime, you should push a new version without the types key in package.json so as not to give people bad types automatically.

@MrXyfir MrXyfir mentioned this issue Jun 3, 2019
@maxlath
Copy link
Owner

maxlath commented Jun 5, 2019

new version without the types: v7.0.8

@r3mi
Copy link

r3mi commented Jan 15, 2020

Hello,
is there a status on TypeScript typings ?
looking around in the issues, it seems there used to be typings but they have been deleted ? It seems also there was typings on DefinitelyTyped but they have been deleted as well ?
is there an update on this ?

Thanks for your library ,

@maxlath
Copy link
Owner

maxlath commented Jan 16, 2020

@r3mi types were added for v6, but then v7 brought some breaking changes and those types became obsolete so were removed. I don't see any breaking change coming, so if anyone wants to update types for v7, I would be ok to merge them back. I still don't use Typescript for now, so I can't do the maintenance of those, but it seems that enough people are interested to make it happen and keep those updated.

@EdJoPaTo
Copy link
Contributor

EdJoPaTo commented May 6, 2020

I created the wikibase-types npm package for types returned by the wikibase / wikidata API. Its helpful but still not as good as a fully typed library.

If someone has the time: The typings from #69 could be used to type this library on DefinitelyTyped but without this change the typings seem to get way more complicated than in #69. (Maybe I have overlooked something?)

@noinkling
Copy link
Contributor

without this change the typings seem to get way more complicated than in #69. (Maybe I have overlooked something?)

That seems weird to me, usually it's not up to the library to write hacks to ensure compatibility between CommonJS and ES modules. I thought that was what the allowSyntheticDefaultImports TS compiler option did (although there are other options that can enable it implicitly), or maybe esModuleInterop.

@EdJoPaTo
Copy link
Contributor

EdJoPaTo commented May 7, 2020

These compiler options only help, as far as i understand, to import things into your lib. The types have to be correct for that in the first place. (Correct me if im wrong!)

The problem in this case is the duplicate export of the common types.
The library exports methods like isPropertyId and the WDK function and the object returned also contains the common methods.

Normally you would create a namespace and the function with the same name and export that to follow the JS code. But you can not export * from from within namespaces which creates a lot of method: typeof method stubs. On the returned interface you have to do the same which creates a lot of typing code just for that. (which is also prone to errors when something changes)

Having the default export you can simply export * from without the need for the extra namespace and have way less typing code. Also i didnt exported the common methods from the returned object which also simplified the typing code a lot (but thats far from perfect typings).

Personally I think the time invested to type this could be also spent to refactor things. But thats up to everyone themselves. I only started to look differently onto source code since I started using TypeScript and I am not used to code which is used in front end / browsers. The way this library is built certainly has its purpose.

@maxlath
Copy link
Owner

maxlath commented Feb 4, 2023

Closing in favor of #82

@maxlath maxlath closed this as completed Feb 4, 2023
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.

5 participants