manzt / zarrita.js

A JavaScript toolkit for working with chunked, compressed, n-dimensional arrays
https://zarrita.dev
MIT License
39 stars 5 forks source link

Update exports for `@zarrita/ndarray` #130

Closed katamartin closed 7 months ago

katamartin commented 7 months ago

👋 I've been (finally!) playing around with integrating @carbonplan/maps with zarrita. It's been a pretty smooth transition, except when I have a basic import from @zarrita/ndarray

import { get } from '@zarrita/ndarray'

I see the following error

Module not found: Package path . is not exported from package
/Users/katamartin/carbonplan/maps/node_modules/@zarrita/ndarray (see exports field in 
/Users/katamartin/carbonplan/maps/node_modules/@zarrita/ndarray/package.json)

I think this should be resolved by updating the exports for@zarrita/ndarray to point to the correct index files (and match the @zarrita/storage exports!). I haven't actually tested this, so I very well may be missing something here!

manzt commented 7 months ago

Thanks for making this fix, although I think this will make sure the imports/types work within the zarrita repo but shouldn't affect what is published to NPM. We use publishConfig to override exports when publishing to npm (more info)

    "publishConfig": {
        "main": "dist/src/index.js",
        "exports": {
            ".": {
                "types": "./dist/src/index.d.ts",
                "import": "./dist/src/index.js"
            }
        }
    }

Are you installing from zarrita@next?

katamartin commented 7 months ago

Oooh interesting, wonder what I'm hitting. Yeah I installed zarrita@next and am specifically using zarrita@^0.4.0-next.6 and @zarrita/ndarray@^0.1.0-next.6

manzt commented 7 months ago

I'll try a fresh import myself and let you know what I figure out...

manzt commented 7 months ago

Ok, took a look into this. The issue is that the package built with microbundle from @carbonplan/maps generates a CommonJS package (module system used in Node) and a modern ESM package (module system used in browsers).

Zarrita only ships ESM, and Next.js resolve the import for @carbonplan/maps to main: "dst/index.js", which is CJS and fails due to require-ing ESM:

const zarr = require("zarrita");

I was able to get the demo working in @carbonplan/maps but updating the next config to prefer the module export in package.json over main:

error: cannot run delta: No such file or directory
diff --git a/demo/next.config.js b/demo/next.config.js
index 5b7a483..95f4633 100644
--- a/demo/next.config.js
+++ b/demo/next.config.js
@@ -4,6 +4,9 @@ module.exports = {
   webpack: (config, options) => {
     if (options.isServer) {
       config.externals = ['react', 'theme-ui', ...config.externals]
+      config.resolve.mainFields = ['module', 'main'];
+    } else {
+      config.resolve.mainFields = ['browser', 'module', 'main'];
     }
     config.resolve.alias['react'] = path.resolve(
       __dirname,

I believe if carbonplan/maps switched to shipping ESM only as well it should not be an issue. For code that is intended to run in the browser-like environments, I don't see the need to CJS since it is node-specific and bundlers all target ESM. Happy to help with that transition if it is of interest.

katamartin commented 7 months ago

@manzt thanks for looking into this and digging into our build steps 😅 !

I've had to put this down to work on other things, but I did quickly try updating the Next config in a different consumer repo and couldn't get it working. I started to play with transitioning to ESM based on your suggestion, but ran out of time to fully test that out. Hoping to get back to this soon with these breadcrumbs -- thanks again for all of the help!