wikibase-sdk icon indicating copy to clipboard operation
wikibase-sdk copied to clipboard

Missing types library

Open trobert2 opened this issue 2 years ago • 4 comments

  • wikibase-sdk version: 0.3.0
  • Environment NodeJS: v16.15.1
  • Typescript version: 4.7.3

After importing the library:

import wbk from 'wikibase-sdk'

We get this error:

Could not find a declaration file for module 'wikibase-sdk'. '/Users/trobert2/workspace/gigi/serverless/external-api-services/node_modules/wikibase-sdk/lib/wikibase-sdk.js' implicitly has an 'any' type.
  Try `npm i --save-dev @types/wikibase-sdk` if it exists or add a new declaration (.d.ts) file containing `declare module 'wikibase-sdk';`ts(7016)

Trying to install the types associated with the library fails:

$ npm i --save-dev @types/wikibase-sdk
npm ERR! code E404
npm ERR! 404 Not Found - GET https://registry.npmjs.org/@types%2fwikibase-sdk - Not found
npm ERR! 404 
npm ERR! 404  '@types/wikibase-sdk@*' is not in this registry.
npm ERR! 404 
npm ERR! 404 Note that you can also install from a
npm ERR! 404 tarball, folder, http url, or git url.

npm ERR! A complete log of this run can be found in:
npm ERR!     /Users/trobert2/.npm/_logs/2022-06-15T13_21_37_923Z-debug-0.log

trobert2 avatar Jun 15 '22 13:06 trobert2

see #61

EdJoPaTo avatar Jun 15 '22 15:06 EdJoPaTo

hi @EdJoPaTo, Thanks for the link. Would you be so kind to clarify how that issue is resolved? To me it looks like the conclusion is not to use Typescript in that thread. Is there anything else to it? I saw that wikibase-types exists which is nice for importing interfaces that might be used for response types for example, but that is decoupled from this library so it doesn't help with the importing issue I am mentioning What am I missing here?

trobert2 avatar Jun 15 '22 15:06 trobert2

I think that is the current situation there sadly… I would like TypeScript types a lot but personally I don't have the time for doing a fork or something like that.

Maybe @mshd can provide more info on the current TypeScript situation with Wikidata as packages like @entitree/helper exist (which I sadly havnt checked out further yet).

EdJoPaTo avatar Jun 15 '22 15:06 EdJoPaTo

I'll be busy on another project until October 2022, but was thinking to take some time then to bring back types for wikibase-sdk and possibly also wikibase-edit, as that's now the most demanded feature

maxlath avatar Jun 15 '22 18:06 maxlath

maybe this 2MB wikidata-types file can help: https://github.com/codeledge/entitree-monorepo/blob/main/packages/entitree-helper/dist/index.d.ts

sebilasse avatar Oct 21 '22 04:10 sebilasse

maybe this 2MB wikidata-types file can help: https://github.com/codeledge/entitree-monorepo/blob/main/packages/entitree-helper/dist/index.d.ts

Oh damn, I didn't realize it's 2MBs. If we can can get full type safety for every property that'd be great. It would look something like this file. But I haven't had the time to make it production ready and the other thing is, it would need to be updated often.

mshd avatar Oct 21 '22 04:10 mshd

I converted the codebase to Typescript and published a new major version (v9) with types :tada: As that's a first for me, your feedback is very welcome! Beware, I used the occasion to do some cleanup: there are quite some breaking changes.

maxlath avatar Feb 04 '23 22:02 maxlath

In particular, I couldn't find a way around defining a Wbk interface, where all the wbk functions types are redefined, which is duplicating types already set in the functions individual files (ex: getEntities types are already defined here)

maxlath avatar Feb 04 '23 22:02 maxlath

I played around with the new v9 and its awesome to have types now!

The use of the sub-export wikibase-sdk/wikidata.org is a neat idea!

I have a few ideas and improvements to the types I will just point out here. A bunch of these ideas are probably breaking changes but I think they are worth looking into.

First of all: I thought about providing some as Pull Request but the main branch does not seem to work out for me currently, typescript cant compile. (src/helpers/simplify_text_attributes.ts:26:3 - error TS2322: Type '{}' is not assignable to type 'SimplifiedSingleTermDictonary'.)


One of the main things that would be helpful are exported types. Currently I had to use code like this (to create the searchEntities function with fetch):

import type {SearchResult} from 'wikibase-types';
import {wdk} from 'wikibase-sdk/wikidata.org';

type Wbk = typeof wdk;
type SearchEntitiesOptions = Parameters<Wbk['searchEntities']>[0];

export async function searchEntities(options: SearchEntitiesOptions): Promise<SearchResult[]> {
  const url = wdk.searchEntities(options);
  const response = await fetch(url);
  const body = await response.json() as {search: SearchResult[]};
  return body.search;
}

I would like to just use this:

import type {SearchResult, SearchEntitiesOptions} from 'wikibase-sdk';
import {wdk} from 'wikibase-sdk/wikidata.org';

export async function searchEntities(options: SearchEntitiesOptions): Promise<SearchResult[]> {
  const url = wdk.searchEntities(options);
  const response = await fetch(url);
  const body = await response.json() as {search: SearchResult[]};
  return body.search;
}

Probably my whole wikibase-types package should be a copy, paste & export in the wikibase-sdk. Personally I also think the isItemId methods are a bit more helpful in my wikibase-types package as they return input is string which is neat for type checking on input of any kind.


Move instance independent methods (for example simplify) out of the Wbk thingy.

As you mentioned there is a lot of duplicate function header currently. They do not use the instance in any way so they can be on their own.

I do like the idea of grouping them together but I also like them as prefixed methods. I would just export them plainly so an import would look like this:

import {wdk} from 'wikibase-sdk/wikidata.org';
import {simplifySparqlResults} from 'wikibase-sdk';

Thats easy to use and way less complex.

If they should still be grouped together something like this could be established (instead of the current interface):

// ./src/simplify/bla.ts
export function simplifySparqlResults(…): … {…}

// ./src/simplify/index.ts
export simplifySparqlResults as sparqlResults from './bla.ts';

// ./src/index.ts
export * as simplify from './simplify/index.ts';

Maybe the current Wdk interface could be a class. There is a generator function to create it which would be a constructor and methods on it. Havnt tested anything about that idea, just an idea.


The package.json exports key is currently without the types. TypeScript docs about it

I would expect something like this in the package.json:

   "main": "./dist/index.js",
   "exports": {
-    ".": "./dist/index.js",
-    "./wikidata.org": "./dist/wellknown/wikidata.org.js"
+    ".": {
+      "types": "./dist/index.d.ts",
+      "import": "./dist/index.js"
+    },
+    "./wikidata.org": {
+      "types": "./dist/wellknown/wikidata.org.d.ts",
+      "import": "./dist/wellknown/wikidata.org.js"
+    }
   },
-  "types": "dist/index.d.ts",
+  "types": "./dist/index.d.ts",

ES Modules were experimental in Node.js 12. As ES modules are a requirement now this package probably requires Node.js 14.


I would add some flags to the tsconfig.json. I dont know if they all compile with the current code (the main branch doesnt compile now) but they are somewhat helpful and improve code quality:

{
  "strict": true,
  "noImplicitReturns": true,
  "noImplicitOverride": true,
  "noUnusedLocals": true,
  "noUnusedParameters": true,
  "noFallthroughCasesInSwitch": true,
  "noUncheckedIndexedAccess": true,
  "noPropertyAccessFromIndexSignature": true,
  
  "noEmitOnError": true,
  "forceConsistentCasingInFileNames": true,
  
  "sourceMap": true,
}

Source maps (the last one) are always neat to have especially when the code base is bigger. You can use them with node --enable-source-maps file.js quite easily. Publishing them will allow users of the code to use them too.


I will leave this here for discussion for now. There might be more things I find when migrating to v9 but there already is a bunch of things to think about.

Hopefully I can fully replace my packages (wikibase-types and wikidata-sdk-got) with wikibase-sdk and fetch now and use it easily on both Node.js and Deno. That would be awesome and seems to be in reach. Thanks for your efforts on migrating to TypeScript!

EdJoPaTo avatar Feb 09 '23 17:02 EdJoPaTo

Another thing would be to make all the function options readonly. This way the caller can be sure the content is not changed.

 export interface GetEntitiesOptions {
-  ids: string | string[];
-  languages?: WmLanguageCode | WmLanguageCode[];
-  props?: Props | Props[];
-  format?: UrlResultFormat;
-  redirects?: boolean;
+  readonly ids: string | readonly string[];
+  readonly languages?: WmLanguageCode | readonly WmLanguageCode[];
+  readonly props?: Props | readonly Props[];
+  readonly format?: UrlResultFormat;
+  readonly redirects?: boolean;
 }

EdJoPaTo avatar Feb 09 '23 18:02 EdJoPaTo

Found some inconsistent parameters of wdk.simplify.sparqlResults: the second argument is missing (The one with {minimize: true}). simplifySparqlResults still has the second argument so it got lost on the interface.

(#91 is similar but on different methods)

EdJoPaTo avatar Feb 09 '23 19:02 EdJoPaTo

Except my request for readonly options I think everything here is solved or has an open PR. Maybe close this issue?

EdJoPaTo avatar Feb 19 '23 15:02 EdJoPaTo

Except my request for readonly options I think everything here is solved or has an open PR. Maybe close this issue?

First off, thank you very much for the effort, @EdJoPaTo ! I haven't gotten a chance to personally try out the changes, but I'll do. I'll open another issue if something comes up. 👍

trobert2 avatar Feb 20 '23 10:02 trobert2