0no-co / wonka

🎩 A tiny but capable push & pull stream library for TypeScript and Flow
MIT License
709 stars 29 forks source link

Add exports willdcard using new (Node 17+) notation #146

Closed dchymko closed 1 year ago

dchymko commented 1 year ago

The issue in #97 (['ERR_PACKAGE_PATH_NOT_EXPORTED')appears to be back in Node 18.

I believe that Node 17 deprecated the "./": "./" wildcard notation in the exports section of package.json, so I've added it using the new style.

kitten commented 1 year ago

I haven't tested Node 18 just yet, but maybe it's better to just remove it? I don't think the wildcard export is really needed anymore, and is more a legacy leftover from older versions 😅

dchymko commented 1 year ago

I haven't tested Node 18 just yet, but maybe it's better to just remove it? I don't think the wildcard export is really needed anymore, and is more a legacy leftover from older versions sweat_smile

That's what I thought too, but changing that is the only way I could get it to work with a Rescript project after bumping to node 18.14.0

kitten commented 1 year ago

So, I've checked out the legacy branch again and tested it with Node 18. Unfortunately, I can't find a way to reproduce the exact error you're seeing, so without a reproduction I'd be more on the side of removing the catch-all entry, since its support seems to have entirely been removed now anyway.

ERR_PACKAGE_PATH_NOT_EXPORTED

This is basically what I'm getting when I try anything that the catch-all path allowed before, so I'd remove it either way.

dchymko commented 1 year ago

So, I've checked out the legacy branch again and tested it with Node 18. Unfortunately, I can't find a way to reproduce the exact error you're seeing, so without a reproduction I'd be more on the side of removing the catch-all entry, since its support seems to have entirely been removed now anyway.

ERR_PACKAGE_PATH_NOT_EXPORTED

This is basically what I'm getting when I try anything that the catch-all path allowed before, so I'd remove it either way.

Hmmm, yeah. I was able to reproduce with a basic rescript-project-template. But I found another problem. If this PR is used with Node 16, there's a different error since it doesn't support the newer target: :disappointed:

Error [ERR_INVALID_PACKAGE_TARGET]: Invalid "exports" target "./*" defined for './' in the package config /home/dc/src/sandbox/rescript-project-template/node_modules/wonka/package.json

I think maybe the better solution for us is to just use v6.2.3 of this package and add in the bindings that we use.