Modules marked as external should always be treated as external

[matthewp]

Changes

Previously node-fetch was always being bundled. This was because, as the thought goes, it's small so we should never externalize it.

However if a config explicitly marks it as external (rather than being implicitly external through a dependency) we should honor that.

Testing

Adds a test for the scenario.

Docs

N/A, bug fix.

[vercel]

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/pikapkg/snowpack/9uvvr5HXepXaAyeadZf8UW6Ez8p6
✅ Preview: https://snowpack-git-external-node-fetch2-pikapkg.vercel.app

[natemoo-re]

ext here meaning external? I would probably rename that variable for clarity.

[natemoo-re]

Not sure I totally understand where the previous behavior was letting node-fetch through, but this is a welcome change nonetheless. This is significantly easier to read through.

[matthewp]

@natemoo-re it was coming through because it was filtering the full list of externals. This change makes it so that it only filters the package.json dependencies (and peer and dev) lists. The externals coming from your config are not filtered any more, so that means we always honor your config choices.

[parkerault]

@natemoo-re it was coming through because it was filtering the full list of externals. This change makes it so that it only filters the package.json dependencies (and peer and dev) lists. The externals coming from your config are not filtered any more, so that means we always honor your config choices.

I don't entirely understand what this means; I'm using yarn workspaces with packages built with snowpack, and I have package A which uses package B and C as dependencies. Package B and C had react as peerDependencies, and used external: ["react"] in their snowpack configs. React context only works within the boundaries of a single component tree, so if you have a dependency that exports a context created by its own bundled react, it won't work with the host application. I spent several hours trying to figure out why this wasn't working with react as a peer dependency of the imports, and a dependency of the host. I randomly tried moving react to devDependencies in the package.json of the libraries, and it started working. Is that expected behavior? It's exactly the opposite of what I expected. Let me know if I should post a reproduction.

Edit: I had a different version of snowpack in the host app's package.json. Updating it to the same version in the dependencies seems to have fixed the issue.