kenchris / urlpattern-polyfill

URLPattern polyfill
https://www.npmjs.com/package/urlpattern-polyfill
MIT License
268 stars 31 forks source link

Update package.json exports #64

Closed sgwilym closed 2 years ago

sgwilym commented 2 years ago

The current setup in package.json looked problematic to me, and was causing errors when this package was required.

kenchris commented 2 years ago

@justinfagnani maybe you could review this? Or someone on your team?

justinfagnani commented 2 years ago

Main pointing at .cjs and removing "type": "module" are probably a debatable choices. If this package is primarily intended to be loaded into browsers, then I'm not even sure CJS should be published at all. What's the use case?

sgwilym commented 2 years ago

I'm writing some code for an abstraction which can be used between different kinds of servers. Part of that is doing some pattern matching on URLs found on requests.

Deno has URLPattern, but Node doesn't — and I'd like my code to run on Node as well.

justinfagnani commented 2 years ago

Node 12 and up can import JS modules from other JS modules. If you need to import into CommonJS, you can put a package.json with "type": "commonjs" in a dist/cjs/ folder. That would leave the rest of the package as standard JS modules.

sgwilym commented 2 years ago

The removal of "type": "module" has not stopped the rest of the module from being ESM / UMD, it's just stopped CommonJS files from being interpreted as ESM. These changes shouldn't affect anyone using import syntax, as any tooling from the past few years will be using the module key anyway (this is why main should point to a CommonJS file — only Node cares about it). And lots of tools (i.e. bundlers) will use the named exports over anything else.

I don't really understand what you're suggesting I do — modify the build output locally? I just want to use the package normally.

justinfagnani commented 2 years ago

The CommonJS files already won't be interpreted as JS modules if they have a .cjs extension or are in a folder with a package.json with "type": "commonjs". If you have require export map entries, the any require() will use those. I'm suggesting that this package use one of those ways to denote the CommonJS build, rather than remove the "type": "module", which controls how .js files are interpreted.

sgwilym commented 2 years ago

You seem to have much stronger ideas than I do about how this should be fixed, so I'll just post an issue.

justinfagnani commented 2 years ago

Basically, if you keep the exports map you have, but don't remove "type": "module" and don't change main, it should still work.

sgwilym commented 2 years ago

https://github.com/kenchris/urlpattern-polyfill/issues/65

justinfagnani commented 2 years ago

Ok, I'll take this on