fix: demote browser export

[penx]

Prefer require, import and types exports over browser.

Exports are resolved in order, i.e. the first match is used.

I'm not sure why the browser export is using the raw src, however when Jest is configured with testEnvironment: "jsdom", it will resolve to this file causing 2 issues:

• Jest will not transpile it unless added to transformIgnorePatterns or experimental ESM modules support is enabled
• Jest throws a type error as discussed on @remix-run/web-fetch causes tests to error after upgrade to jest 28 remix#3402

Also:

"types" - can be used by typing systems to resolve the typing file for the given export. This condition should always be included first.
https://nodejs.org/api/packages.html

[changeset-bot]

🦋 Changeset detected

Latest commit: 9105002

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@remix-run/web-fetch	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[MichaelDeBoey]

CC/ @chaance @jacob-ebey @mjackson
Since I can't add you as a reviewer (I don't have any permissions on this repo), I wanted to tag you guys.

[JRasmusBm]

I've been digging for over two hours and this is clearly the fix. Is there any progress on this PR? 😇

EDIT: Solving it with patch-package for now.

[nitrog7]

nudge

[brophdawg11]

Exports are resolved in order, i.e. the first match is used.

Is there a source/reference for this?

[penx]

Exports are resolved in order, i.e. the first match is used.

Is there a source/reference for this?

Within the "exports" object, key order is significant. During condition matching, earlier entries have higher priority and take precedence over later entries. The general rule is that conditions should be from most specific to least specific in object order.

https://nodejs.org/api/packages.html#conditional-exports

[jacob-ebey]

I don't believe this is "the fix" everyone is after here. Generally, any non-standard conditions like "browser" are put above the "require", "import", and "default" conditions. Reasoning being some bundlers (like esbuild) don't allow you to disable the default conditions, and instead configure conditions to check before the defaults.

This honestly sounds like a Jest issue of loading the wrong conditions for where they are executing. You aren't actually running Jest in the browser, so they should not be attempting to load the browser condition.

Moving browser below the standard conditions will mean that the "browser" condition will never be picked, even when building for / targeting the browser.

[brophdawg11]

Thanks @penx! I missed that when I looked through those docs.

I agree with @jacob-ebey here - that snippet sounds to me like browser belongs above import/require as it's more specific. Just like node should be above per the docs.

It does feel like Jest either (1) should not pick up the browser entry or (2) should handle ESM due to type:module.

I think we should split off the moving types to the top into a new Pr and merge that since that feels cut and dry. Moving browser around feels wrong and likely requires some more investigation into exactly why jest can't handle it correctly.

[brophdawg11]

Extracted the lifting of types to the top into #38

[MichaelDeBoey]

Instead of demoting the browser export, we should provide a CJS version for browser as well, which is exactly what I did in #43