improve our CJS scanner

[FredKSchott]

Changes

• cjs-module-lexer isn't the magic bullet that we'd hoped it would be. Many popular packages continue to fail static analysis, and Rollup has recently made a backwards incompatible change that will cause all packages built with Rollup to fail scanning, going forward.
• Meanwhile our runtime analyzer continues to run well, with our namedExports opt-in support fixing most issues in the scanner.
• This change now runs the runtime namedExports scanner on all CJS packages. Because they're CJS, they are expected to run in Node.js without error. But, on the off chance that runtime analysis fails, we will silently continue and you won't be any worse off then you were before.
• This may make the installer a bit slower on large projects, but since installing is a one-time cost we're okay to pay it. And, we assume that the number of CJS packages will only continue to drop, meaning eventually this step can phase out of the installer entirely.

Testing

• Tests updated.

Docs

• Docs updated.

[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/CkQP7YzDYJv36HBxsypCtffigvpz
✅ Preview: https://snowpack-git-better-cjs-support-pikapkg.vercel.app

[FredKSchott]

/cc @BPScott this was the feature I'd mentioned on Friday. CJS support should get a lot more reliable with this PR.

[BPScott]

Edge case: Looking at node docs I don't think just the existence of an exports field is enough to prove that the package should be treated as esm.

It is supported in Node.js 12+ as an alternative to the "main" that can support defining subpath exports and conditional exports while encapsulating internal unexported modules.

I think the following should be treated as exposing commonjs:

{"exports": "./main.js"} //omiting type defaults to type being commonjs

{"exports": "./main.js", "type": "commonjs"}

{"exports": "./main.cjs", "type": "module"}

I think a comprehensive check for this needs to take into account the type field and the extension used in the exports field.

I think this gets further complicated as "is this ESM" is something that needs to happen on a single entrypoint level rather than across a whole package. But a larger refactor for this can totally be punted. E.g. I could create exports where one entrypoint is exposed as esm, and another entrypoint is cjs only:

 "exports": {
    ".": "./main.cjs", // cjs only
    "./subpath": "./secondary.mjs", // mjs only
    "./another-subpath": "./secondary.js", // To know if this is esm/cjs, check the type field
}

I could probably take this to pathological obnoxiousness by then introducing conditional exports, but I'm already falling too far down the rabbit hole for this PR :)

[FredKSchott]

Good call, even if unlikely you're right that it's not impossible. Luckily I think that we can remove this entirely now with the better CJS handling in place.

[BPScott]

Lovely stuff! Commented on what I think might be an incorrect assumption inline.

I mentioned this on Friday, and I'm sure you've wrote it down somewhere already but mentioning here for posterity: I think it'd be neat if logging the package info included if the package was CJS. When integrating snowpack into an existing application I think it'd be useful to be able to see at a glance which packages are CJS and thus may result in FUN when importing them. (And thus which packages you check for updates to see if newer versions ship esm)

[ryanatkn]

This didn't seem worth opening an issue: this PR introduces the execa dependency to esinstall here for the first time, but it wasn't added to package.json, so the latest version throws a runtime error if it's not installed. Thank you for everything!