cjs auto-detection on by default #1163
Conversation
|
This pull request is being automatically deployed with Vercel (learn more). 🔍 Inspect: https://vercel.com/pikapkg/snowpack/r3kzf0yiw |
FredKSchott
force-pushed
the
wip-cjs-autodetect
branch
from
b14bda5 to
a62a64d
Compare
FredKSchott
force-pushed
the
wip-cjs-autodetect
branch
from
a62a64d to
0264967
Compare
| */ | ||
| function cjsAutoDetectExportsRuntime(normalizedFileLoc: string): string[] | undefined { | ||
| try { | ||
| return Object.keys(require(normalizedFileLoc)).filter((imp) => imp !== 'default'); |
There was a problem hiding this comment.
Do we need to detect if require() isn’t an object with keys first (e.g. module.exports = myFunc() or module.exports = 'some string')? I guess that would fall into the catch() block but it may be a bit noisy
There was a problem hiding this comment.
ah, yea that's a good call. Yea that would be a "default export" anyway so the failure gives us the result we want, but yea I'd rather code defensively.
| @@ -0,0 +1,2 @@ | |||
| exports.export1 = 'foo'; | |||
| exports.export2 = 'bar'; | |||
There was a problem hiding this comment.
Not blocking this PR, but it’d be great to test module.exports = [various things that may conflict / compete with the other exports] either in the same package or another package
There was a problem hiding this comment.
since we're falling back to https://github.com/guybedford/cjs-module-lexer for this analysis, we can assume the edge cases are already tested on their end. But, I agree we could do a bit more here than just this simple example, will add a few more lines/exports.
| export2: export2 | ||
| }; | ||
| export default entrypoint; | ||
| export { entrypoint as __moduleExports, export1, export2 };" |
There was a problem hiding this comment.
Interesting! It’s fun to see things progress to interop better between CJS and ESM
|
This is such a huge addition! I feel like this has been historically one of the biggest mental barriers to using Snowpack. |
FredKSchott
force-pushed
the
wip-cjs-autodetect
branch
from
0264967 to
84a5466
Compare
FredKSchott
force-pushed
the
wip-cjs-autodetect
branch
from
84a5466 to
6ff33d2
Compare
Changes
exports.foo = require('./foo.js')).Testing