Skip to content

Commit

Permalink
feat: support conditional exports in js client (#963)
Browse files Browse the repository at this point in the history
## Description of changes
Currently not all bundlers will chose the correct CJS/ESM build when
importing the js client because we don't define them using
[conditional-exports](https://nodejs.org/api/packages.html#conditional-exports).
This change adds the required configuration in the package.json to fix
that.

 - Improvements & Bug fixes
	 - support conditional exports in js client


## Test plan
I have tested this manually. We have a Next.js setup which throws an
error with v1.5.6, we make use of that to detect which build (cjs/esm)
nextjs tries to load. In both cases we expect an error with the path
from which nextjs loads the module.


### Reproduce the issue
- checkout
jeffchuber/nextjs-chroma@2df1a21
- run yarn && yarn dev
- open http://localhost:3000/
- you should see the following error in the browser
```
Failed to compile

./node_modules/chromadb/dist/main/embeddings/WebAIEmbeddingFunction.js:157:0
Module not found: Can't resolve '@visheratin/web-ai'

https://nextjs.org/docs/messages/module-not-found

Import trace for requested module:
./node_modules/chromadb/dist/main/index.js
./src/app/page.js

This error occurred during the build process and can only be dismissed by fixing the error.
```
Nextjs loads the cjs build from `dist/main`.

### Confirm the change resolves the issue
- checkout
jeffchuber/nextjs-chroma@2df1a21
- remove current chromadb package (1.5.6) using `yarn remove chromadb`
- install chromadb client with the [change in this
pr](450159e)
using package linking or using verdaccio
- open http://localhost:3000/
- you should see the following error in the browser
```
Failed to compile

./node_modules/chromadb/dist/module/embeddings/WebAIEmbeddingFunction.js:131:0
Module not found: Can't resolve '@visheratin/web-ai'

https://nextjs.org/docs/messages/module-not-found

Import trace for requested module:
./node_modules/chromadb/dist/module/index.js
./src/app/page.js

This error occurred during the build process and can only be dismissed by fixing the error.
```
Nextjs now loads the esm build from `dist/module`

## Documentation Changes
I added a reference to
https://nodejs.org/api/packages.html#conditional-exports in the commit
message
  • Loading branch information
perzeuss authored Aug 16, 2023
1 parent 4116cad commit f8186ff
Showing 1 changed file with 5 additions and 1 deletion.
6 changes: 5 additions & 1 deletion clients/js/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,11 @@
"typescript": "^5.0.4"
},
"main": "dist/main/index.js",
"module": "dist/module/index.js",
"module": "dist/module/index.js",
"exports": {
"require": "./dist/main/index.js",
"import": "./dist/module/index.js"
},
"files": [
"src",
"dist"
Expand Down

0 comments on commit f8186ff

Please sign in to comment.