hildjj / node-cbor

Encode and decode CBOR documents, with both easy mode, streaming mode, and SAX-style evented mode.
MIT License
357 stars 73 forks source link

`node-inspect-extracted` missing in dependencies #141

Closed letmaik closed 3 years ago

letmaik commented 3 years ago

The cbor package imports node-inspect-extracted but doesn't declare it as dependency.

hildjj commented 3 years ago

You've got a solid point here. My reasoning was that node users would never see this require, and that browser users would be using cbor-web.

To fix this, I want to understand the use case. Are you packaging cbor yourself?

letmaik commented 3 years ago

Yes I'm packaging cbor myself with rollup, which essentially lifts CommonJS to ESM in order to tree-shake. A side-effect of this is that all imports get hoisted to the top as there are no synchronous conditional imports in ESM. This is where the error is coming from. I could use cbor-web, but I'd prefer not to given that it bundles dependencies, which I'd like to leave as a concern for my own application bundling and hence package-lock.json.

hildjj commented 3 years ago

I don't think making node-inspect-extracted an optional peer dependency really captures the gist of what's wanted here, which is that node users don't ever see the dependency. I'm thinking about adding a plugin API anyway, because of #140.

I'm open to suggestions.

letmaik commented 3 years ago

I think what you're looking for is https://nodejs.org/api/packages.html#packages_subpath_imports

hildjj commented 3 years ago

not-quite-related: https://github.com/hildjj/node-inspect-extracted/issues/8

hildjj commented 3 years ago

Subpath imports require node 12. I'm not quite ready to pull that trigger yet; I'm waiting to see how https://gist.github.com/sindresorhus/a39789f98801d908bbc7ff3ecc99d99c plays out

letmaik commented 3 years ago

node 14 is the current LTS and node 10 is EOL and won't be security patched anymore. Any reason you want to keep supporting node 10?

hildjj commented 3 years ago

You're of course correct, but based on the amount of heartache that I got from turning off support for Node 6, I want to wait a month or two. I'll also think about moving to ES6 modules and doing a major release at that time.

lukem512 commented 3 years ago

+1 for this. I've had to use cbor-web for the moment but it doesn't feel like a nice solution.

letmaik commented 3 years ago

I've switched to https://github.com/aaronhuggins/cbor-redux for now as it's very lightweight (no dependencies) and can run in any JS environment. It would be nice if node-cbor eventually is restructured to support such use cases. I realize there are some challenges with making this compatible with the streaming interface but I have a feeling most people will use the simple interface anyway?

hildjj commented 3 years ago

I've got a much simpler interface currently in https://github.com/hildjj/cbor-wasm but it's not done yet.

letmaik commented 3 years ago

Interesting, in my case I would also need an encoder and can't run wasm currently.

hildjj commented 3 years ago

Update: I'll do the update to require node 12 soon after Ava releases their 4.0 version.

hildjj commented 3 years ago

Please retest with v8.0.0 and reopen if this isn't fixed.