gh-43: Full support for ESM modules. #56
Conversation
| @@ -7,9 +7,9 @@ | |||
| "types": "./dist/index.d.ts", | |||
| "exports": { | |||
| ".": { | |||
| "types": "./dist/index.d.ts", | |||
There was a problem hiding this comment.
types should always go first. See typescript doc
| ], | ||
| "scripts": { | ||
| "test": "tsc -noEmit -p tsconfig-test.json && jest --useStderr --runInBand --detectOpenHandles", | ||
| "build": "npm run lint && tsup src/index.ts --format cjs,esm --dts --clean --platform neutral --minify", | ||
| "build": "npm run lint && tsup src/index.ts --format cjs,esm --dts --clean --platform neutral --minify && cp ./dist/index.d.ts ./dist/index.d.mts", |
There was a problem hiding this comment.
Simply duplicate declaration file with d.mts extension. This will make typescript compiler happy because for .mjs module types will be visible.
|
@iSuslov the intention of making this package a CommonJS library is to keep backwards compatibility with the projects who previously used the CommonJS JavaScript client which this client was migrated from. Does changing this package type to module break this compatibility? |
|
To avoid any confusion in the future about your contribution to Weaviate, we work with a Contributor License Agreement. If you agree, you can simply add a comment to this PR that you agree with the CLA so that we can merge. |
|
Hi @iSuslov 😁 what is the status of this PR? Are you in a position to reply to #56 (comment)? |
Although I am not the submitter of this PR, I can reply to your comment: if you change the type to module in the package.json, this will be a breaking change. Although you can still import an ESM module into a CommonJs application, it has to be done using the dynamic import function, since ESM modules have asynchronous execution. See here for more information. Still, all things considered, I would rather have you move to ESM than using the approach suggested here using an additional build step. Many npm packages have already migrated to ESM, and although it is a bit of a pain to move your code to ESM, it is not rocket science, and you get other benefits too. |
Fixes #43 in
v1.3Problem:
In typescript when using more strict
moduleResolutionsuch asnodenextornode16, typescript can't find type declarations of the esm module which has.mjsextension. This is because TypeScript ignores.jswhen there is an identically named.d.tsfile beside it. Same happens with.mjsand.d.mtsbut if no such pair found, typescript thinks there are no type declarations.More info in my comment here.
Solution:
Assuming that weaviate team doesn't want to change package module type from
commonjstomodule, keepingcommonjsas default option, the smallest change is to generated.mtsdeclaration file. Since we don't have actual.mtsfiles, it's not easy to emit one using standard bundlers and builders, see. The best workaround I see is to clone original type definitions after build. This will make this PR small and if one day weaviate team decides to implement a build pipeline it will be straightforward to implement this step using other tools.