Export URLPattern in CJS file too

[fregante]

Both entry points should export the same thing, or else import {URLPattern} only works if the bundler reads the ESM file, which may decide not to.

[SanderElias]

The thing is, you might be ending up exporting the pre-existing implementation. This is not really clean and leaks a fraction of memory. I get the PR from a practical standpoint and have no hard objection to merging it.

[fregante]

The setup is unfortunately still incorrect though. Here's what I suggest:

• dist/index.js acts as a polyfill and has no exports
• dist/index.d.ts only has the line export {} to match the above
• dist/url-pattern.js only exports URLPattern and does not set the global
• dist/url-pattern.d.ts only exports URLPattern and the rest of the types

Using import {URLPattern} currently sets the global, and that's unexpected

[fregante]

The thing is, you might be ending up exporting the pre-existing implementation.

You're right, I fixed that. Now it matches what the ESM version does, even though my previous comment has a better overall solution.

[kenchris]

Users that just used the polyfill (patches global) still probably want the types for using in their IDE. I wonder if we should call it url-pattern.interfaces.d.ts like TS generates

[fregante]

Users that just used the polyfill (patches global) still probably want the types for using in their IDE

That's still fine:

// Polyfill only
import "urlpattern-polyfill"

// Type only
import { URLPatternInit } from "urlpattern-polyfill/url-pattern"

I wonder if we should call it url-pattern.interfaces.d.ts like TS generates

That just makes ugly imports.

Here's what I suggest

The opposite also works, and for me it would look even better, including for the type exports:

// Only import it
import { URLPattern, URLPatternInit } from "urlpattern-polyfill"

// Only polyfill it
import "urlpattern-polyfill/register" // or "urlpattern-polyfill/polyfill"

// And then import just the type
import { URLPatternInit } from "urlpattern-polyfill"

[kenchris]

I will let @SanderElias review and merge, but he is off for the next couple of hours.

Maybe you want to attempt your other suggestion while waiting and we can look at it, but please also update the README.md to make all of this clear :-) Better documentation is always preferred

[SanderElias]

I'll come back to this in a couple of hours.

[kenchris]

The pattern was set up in the way it was, so you would not pay the price for the full payload.

Thinking about it, it's a bit weird that one way of loading acts differently that the other, and especially as we expect most to use modern ESM imports

[fregante]

In ESM there is no way around that, but in commonJS we can.

There is, it's dynamic import()

However still I prefer the solution I suggested in the other comment: never export things and set globals at the same time.

[fregante]

When you really want to get the implementation while using commonJS (node only usually!) you can load it from the URL-pattern.js file.

That's also broken by the way:

Module not found: Error: Package path ./dist/url-pattern.cjs is not exported from package ./node_modules/urlpattern-polyfill (see exports field in ./node_modules/urlpattern-polyfill/package.json)

[fregante]

I opened https://github.com/kenchris/urlpattern-polyfill/issues/85 instead