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

Separate package for FileSystemStore #205

Closed keller-mark closed 3 weeks ago

keller-mark commented 3 weeks ago

The node: dependencies in FileSystemStore complicate usage of @zarrita/storage.

Even if only non-node stores like ReferenceStore are imported using syntax like

import { ReferenceStore } from '@zarrita/storage';

bundlers are not smart enough.

Minimal example: https://stackblitz.com/edit/vitejs-vite-afkkaz?file=src%2Fmain.ts&file=package.json (run npm run build in terminal)

Screenshot 2024-08-23 at 10 58 27 AM

Possible solutions

Can FileSystemStore live in its own sub-pacakge? Or can all other stores be imported into a new sub-package like @zarrita/storage-browser (which would be non-breaking)?

keller-mark commented 3 weeks ago

I am currently using pnpm patch to remove this line, which is another option

https://github.com/manzt/zarrita.js/blob/c7167cb2d16a62786cd5d734615a52acb6652791/packages/storage/src/index.ts#L2

manzt commented 3 weeks ago

Would you be able to do:

import ReferenceStore from "@zarrita/storage/ref";

?

manzt commented 3 weeks ago

I've tried to code-split each store as a separate package entrypoint for this reason, but I'm starting to think it would be easier to implement as separately versioned packages.

keller-mark commented 3 weeks ago

With "moduleResolution": "Node", in my tsconfig.json I cannot, but if I change it to "moduleResolution": "Bundler", I can

manzt commented 3 weeks ago

Ooo thanks for trying, would you be able to try nodenext? Just curious...

keller-mark commented 3 weeks ago

I tried nodenext, but then it tells me I also need to change my module value. I am not sure the optimal tsconfig and what downstream effects changing these properties will have

keller-mark commented 3 weeks ago

For reference, this is the current file: https://github.com/vitessce/vitessce/blob/main/tsconfig.json

manzt commented 3 weeks ago

Got it. Thanks for giving a try for me. I was hoping the entrypoints would work but maybe it's a bit too restrictive and we should just have separate packages.

keller-mark commented 3 weeks ago

"moduleResolution": "Bundler" seems to be working, so maybe that is the best / most modern solution, and can just be documented in the docs site somewhere

keller-mark commented 3 weeks ago

Changing the import statement to use the entry point in the minimal reproducer linked above also resolves the issue so please feel free to close this issue

manzt commented 3 weeks ago

Ok, I'll close for now but feel free to reopen if you find that doesn't work for a future use case!