refactor: remove browser-resolve dependency, inline logic #127
Conversation
| if (browser[importee] === false) { | ||
| return ES6_BROWSER_EMPTY; | ||
| } | ||
| } |
There was a problem hiding this comment.
This is the crux of the replacement logic with browser-resolve. Given a map from an importer - resolve to the values of that key. Here we can also take advantage of returning ES6_BROWSER_EMPTY as early as possible - as we don't need to resolbe that beyond what it is doing.
| if ( !disregardResult && !err ) { | ||
| if ( resolved && fs.existsSync( resolved ) ) { | ||
| resolved = fs.realpathSync( resolved ); | ||
| } | ||
|
|
||
| if ( resolved === COMMONJS_BROWSER_EMPTY ) { | ||
| fulfil( ES6_BROWSER_EMPTY ); |
There was a problem hiding this comment.
We no longer need this line as we're not relying on browser-resolves empty file.
| if (key[0] === '.' && !extname(key)) browser[ key + '.js'] = browser[ key + '.json' ] = browser[ key ]; | ||
| return browser; | ||
| }, {}); | ||
| } |
There was a problem hiding this comment.
This is a convenient side-effect-ful mechanism of the packageFilter function. We know that if we land inside a package.json file, that the resolved importee has this mapping inside of its own package.json. Therefore, if we collect these browser maps inside of a cache (browserMapCache) then by the time we get to the importee that owns this package.json being the importer, all of it's own dependencies can rely on this cache.
In other words - it is this operation that collects for us, the mapping to which files are replaced in a browser based environment.
| if (options.browser && typeof pkg[ 'browser' ] === 'string') { | ||
| pkg[ 'main' ] = pkg[ 'browser' ]; | ||
| } else if (options.browser && pkg[ 'browser' ][ pkg[ 'main' ] ]) { | ||
| pkg[ 'main' ] = pkg[ 'browser' ][ pkg[ 'main' ] ]; |
There was a problem hiding this comment.
This is the second key part of the browser key logic - which is that if we hit a module, it's effective main field is either browser (if it is a string) or browser[pkg.main] (if it is an object). Luckily we can use packageFilter exactly how we already are to acheive this.
|
Tests are failing because Node 6 timed out. https://travis-ci.org/rollup/rollup-plugin-node-resolve/jobs/326432217. Nodes 4 and 9 pass, so I'm sure a rebuild of 6 will pass too. |
|
Really happy with how this turned out. 👍 Thanks for the hard work @keithamus! |
|
Thank you! Released 3.0.1 |
|
Hey there, just letting you know that this broke our build, so it might have been different to the original implementation for the browser field resolver: https://travis-ci.org/algolia/react-instantsearch/builds/327346207#L685-L691 |
|
I'll get a fix for this. |
|
@Haroenv this is now fixed in 3.0.2, thanks to @Rich-Harris 😄 |
This PR removes
browser-resolveas a dependency, inlining the logic steps it makes. It does this for a few reasons:resolvemore.resolve. Dropping this dependency means shipping less code.browser-resolveto fix the above issues - doing so would reducebrowser-resolvedown to a few LOC, and I'm not sure it is under active maintainence. There are open PRs from a few years ago. I figured it would be best to move the few LOC it needs into this module./cc @mislav who has been working with me on this today.