search

weaviate / typescript-client

Official Weaviate TypeScript Client
https://www.npmjs.com/package/weaviate-client
BSD 3-Clause "New" or "Revised" License
65 stars 23 forks source link

gh-43: Full support for ESM modules. #56

Closed iSuslov closed 1 month ago

iSuslov commented 1 year ago

Fixes https://github.com/weaviate/typescript-client/issues/43 in v1.3

Problem: In typescript when using more strict moduleResolution such as nodenext or node16, typescript can't find type declarations of the esm module which has .mjs extension. This is because TypeScript ignores .js when there is an identically named .d.ts file beside it. Same happens with .mjs and .d.mts but 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 commonjs to module, keeping commonjs as default option, the smallest change is to generate d.mts declaration file. Since we don't have actual .mts files, 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.

parkerduckworth commented 1 year ago

@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?

weaviate-git-bot commented 1 year ago

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.

beep boop - the Weaviate bot 👋🤖

PS:
Are you already a member of the Weaviate Slack channel?

tsmith023 commented 1 year ago

Hi @iSuslov 😁 what is the status of this PR? Are you in a position to reply to https://github.com/weaviate/typescript-client/pull/56#issuecomment-1545785442?

erikvullings commented 9 months ago

@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?

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.