Resolved files do not respect the `extensions` option

[dcherman]

Given a browser field like the following

{
  './value': './value.browser'
}

It looks like this lib properly handles the missing file extension on ./value, but does no handling of the file that it maps to (./value.browser). I think if the extensions property was provided and a browser mapping does not exist on the filesystem, we should iterate through the extensions property to see if any of the resulting paths does exist on the filesystem.

Thoughts?

Reproducer: https://github.com/dcherman/jest-browser-reproducer Ref: https://github.com/facebook/jest/issues/6005

[dcherman]

@defunctzombie Any opinions on this? I'm happy to work on a PR if you agree that it's a problem with this library rather than something Jest should be responsible for.

[defunctzombie]

Sounds like something this library should be responsible for - assuming this falls into the realm of "resolve this path for browser bundling".

I would entertain a PR; how often does this occur and is it resolvable by the user adding an extension? I suppose the real question I am asking is: how important is it to support not specifying an extension in the browser mappings? Seems to me that a relatively easy fix is saying that you must specify an extension - tho I can see how one might be inclined to leave it off. I believe the reason the "key" part of the mapping does not require an extension is because we do a simple lookup based on what you use in the require or import string.

Thanks for pinging me (I don't get alerts on this repo and am not active on it otherwise).

[dcherman]

@defunctzombie Users definitely can work around this by adding an extension to the values in the browser mappings, but I think it's a bit unexpected.

If foo maps to bar, then I'd expect that

import myDefaultValue from 'foo';

Would behave identically to

import myDefaultValue from 'bar';

Since they resolve to the identical file after mappings are considered, but that's not the case for what's happening right now.

FWIW, Webpack seems to handle browser mappings in the manner that I'm suggesting since our team only ran into this when attempting to use Jest, but our Webpack builds run just fine. Not sure if they use this lib internally or not, but it seems relevant to the discussion.

[dcherman]

Hey @defunctzombie, just pinging you since I know you said you don't get notifications here.

Based on what I discussed above, I'd suggest that we solve it in this lib. If you agree, I'll work on a PR.

[defunctzombie]

I have re-read the issue and I think what you propose is reasonable. Iterating through the list of extensions and trying those out. 👍 @dcherman

[dcherman]

@defunctzombie It was actually always a bug in this library. When I was trying to write tests for this, I realized that the async path worked correctly while the sync path did not, and it ended up being that we had diverging logic between the two paths.

I corrected that by just copying the logic from the async path and making it sync as well as writing a couple tests for that scenario.