Improve CJS Named Exports support

[FredKSchott]

Original Discussion: https://www.pika.dev/npm/snowpack/discuss/309
/cc @bugzpodder, @FredKSchott

We have users requesting named export support for Common.js packages, usually because these packages documentation is written based on Webpack, and not Node's official ESM support.

This is also important for TypeScript users, where TS actually tells the user to use the named export format, so that they can get type information back.

We should support some config to allowlist Common.js packages for named exports. We already support a allowlist, it's just static. This would be a new config value of an array of install target (ie: package names) that should be supported.

// example
"installOptions": {
  "namedExports": ["react-table", "react-csv"]
}

We should also warn when we detect this usage without a allowlist entry. We can detect this during the source code scan phase, and then log a warning to the user.

[guybedford]

I thought I'd mention here if you're interested in automatic detection I was trying to get this PR into Node.js based on a lexing approach (fork of es module shims!) - nodejs/node#33416.

It's still quite uncertain if that will ever get through, but the wasm library that does the extraction is available here https://github.com/guybedford/cjs-module-lexer, which could be a consideration and saving over requiring users to provide this metadata manually which is always somewhat of a pain.

Certainly pros and cons to be weighed here but if you are interested in integrating the main difficulty is handling the module.exports = require(...) which require some recursive processing eg here https://github.com/nodejs/node/pull/33416/files#diff-76195ce57689942222a27f0dbda6d3b7R1330.

[bugzpodder]

just curious, isn't checking package.json for a few fields good enough (module vs main) vs a lexer?

[guybedford]

I missed that you were taking advantage of Rollup's code analysis process for named exports detection here. In which case, yes please ignore my comment!

[FredKSchott]

Thanks for the links, we actually do have a need for this! Right now, we basically create a fake wrapper that gets sent to Rollup's named export detection: https://github.com/pikapkg/snowpack/blob/master/src/rollup-plugin-wrap-install-targets.ts#L91

But, to generate this list of named exports we use a super naive Object.keys(require(pkgName)): https://github.com/pikapkg/snowpack/blob/master/src/rollup-plugin-wrap-install-targets.ts#L36

Because of the risk of side effects, we keep this as an allowlist of popular packages for now. But, this lexer could help us open this this for all CJS packages, automatically.

@guybedford How reliable is the lexer? Works on any packages, any known issues, etc etc?

[guybedford]

@FredKSchott it's certainly not 100% accurate, in fact about 28% of packages aren't supported under this transform. This is mostly cases like class P { method() }; module.exports = P sort of things though that don't get statically parsed and perhaps don't make sense to. @GeoffreyBooth did some detailed tests in nodejs/node#33416 (comment).

[FredKSchott]

Gotcha, will keep that in mind. Thanks for sharing 👍

[W1U02]

lodash is not working too.

import {memoize, orderBy} from "lodash";

// error in browser
// Uncaught SyntaxError: The requested module '/web_modules/lodash.js' does not provide an export named 'memoize'

[FredKSchott]

Oh, that's a good one to handle. The workaround for now is:

// Option 1
import lodash from "lodash";
lodash.memoize(...

// Option 2
import {memoize, orderBy} from "lodash-es";

// Option 3
// npm/yarn install lodash-es under the alias lodash

[pastak]

eventemitter2 is not working as same as lodash.

'/web_modules/eventemitter2.js' does not provide an export named 'EventEmitter2'

I use workaround for now: Rewriting to default import

[nojaf]

Can anyone confirm that this works when you run snowpack build?
I still get Uncaught SyntaxError: The requested module '../../../web_modules/react.js' does not provide an export named 'useEffect' when using React with snowpack 2.7.2.

[timshannon]

Doesn't seem to be working for me. I thought it was working fine with 2.7.0-pre.1 but that seems to no longer work. I'm not sure what's changed.

[bregydoc]

Hi, when I build my react+typescript snowpack project with this config:

{
  "extends": "@snowpack/app-scripts-react",
  "plugins": [],
  "alias": {
    "@app": "./src/"
  }
}

I obtain Uncaught SyntaxError: The requested module '../../../../web_modules/react.js' does not provide an export named 'useEffect', the same error that nojaf.

[nojaf]

Tracked in #844

[FredKSchott]

From @Papierschiff in #844

Not sure why, but when I set "installOptions": { "treeshake": false } it is working again. perhaps this helps to debug the issue.

^ seems to be a workable short-term fix, while we triage

[osdiab]

hmm... kinda sad to see this - i was trying out moving my company's typescript create-react-app project to snowpack but the lack of support for named exports for CJS libraries means that an overwhelming number of libraries we depend on don't work with snowpack and for a project of our size checking to see if snowpack has an issue with each import by running snowpack dev takes a really long time, not to mention that everytime a teammate adds a library there will be a high chance that this will be a problem, requiring everyone in my team to be aware of these details - which goes against the ethos of "it will just work" ethos of the snowpack website. I'll probably be moving onto the next alternative, but wanted to see out of curiosity if having an option for flexibility to avoid being incompatible with a very large number of npm libraries is on the table.

also, i was trying to deal with these import errors with the io-ts-types library, but that one seems to break regardless if i use namedExports key or not: it has both lib/ (commonjs) and es6/ versions of the library, and the option to import without those prefixes and have your runtime automatically choose whether it will use the commonjs or es6 modules style code. it does this by provide individual package.json files for each module with separate main and module fields, to allow the bundler to decide which import style gets loaded. But snowpack seems to fail both if I don't provide io-ts-types in the namedExports field (named exports not supported), and if I do (there is no default export). I'm guessing this dual commonjs/es6 modules export syntax is probably not jiving well with however snowpack does this resolution.

[FredKSchott]

We're just following Node.js behavior here, but I do get your frustration. It's a technical limitation: CJS named exports are defined at runtime, so can't be statically analyzed.

However, I've heard rumors that Node.js is working on a pre-pass scanner that will do it's best to detect named exports automatically from Common.js packages. We'll wait until that's official before we try to implement ourselves, but that would mean that in a future version MOST common.js packages would support named exports by default.

Many popular packages are ESM now, so hopefully this won't be too hard to handle today. We can also look into adding a warning if we see you doing named imports from a common.js package.

[jsefiani]

I basically experience the same issue with graphql-request. I tried do a default import but that wouldn't work either?
I keep getting this message: The requested module '/web_modules/graphql-request.js' does not provide an export named 'gql', what should I do because I really need that library to work? @FredKSchott

[FredKSchott]

You know what, now that we have that error in the browser, we can add some extra debugging info to it! Something like:

If you believe this to be an error, you can add "graphql-request" to your Snowpack "namedExports" configuration to enable better-named export detection for legacy packages: 

"installOptions": {
  "namedExports": ["graphql-request"]
}

[FredKSchott]

Although, @jsefiani can you confirm that you're on the latest version of Snowpack? We recently added automatic CJS named export detection, and it looks like it should be picking that up:

[jsefiani]

Thank you for the quick reply, that resolved my issue! Nope, I'm not on the latest version, will update it shortly (current version: 2.11.0). Thank again mate!

[FredKSchott]

Awesome! Yea if you update, that namedExports config should no longer be needed. Cheers!

[cherihung]

Sorry. this is closed issue. I posted in a new issue now.
#1311

---

Hi, we are running into a similar issue still using the latest snowpack 2.13.3 with the namedExports support. It's a typescript project. could be an issue with unwrapping exports and mixing of import types. not sure if there's rollup config I can use to address this.

I'm importing a custom styled component library so I declared the exports with something like this:

"installOptions": {
    "namedExports": ["@panda/custom-lib"]
  },

in the app itself, I import the custom styled component:

import { Svg } from '@panda/custom-lib';

When I start up the dev build, I get a Uncaught TypeError: styled_components_1.default.svg is not a function error.
because in runtime with snowpack, the actual default is in styled_components_1.default.default.svg

I suspect the error is likely from the unpacking.
The source, compiled file:

Object.defineProperty(exports, "__esModule", { value: true });
exports.StyledSvg = void 0;
var tslib_1 = require("tslib");
var styled_components_1 = tslib_1.__importDefault(require("styled-components"));
var styled_system_1 = require("styled-system");
exports.StyledSvg = styled_components_1.default.svg(templateObject_1 || (templateObject_1 = 
.....

The runtime file in browser:

var styled = createCommonjsModule(function (module, exports) {
Object.defineProperty(exports, "__esModule", { value: true });
exports.StyledSvg = void 0;

var styled_components_1 = tslib_es6.__importDefault(styledComponents_browser_esm);

exports.StyledSvg = styled_components_1.default.svg(templateObject_1 || (templateObject_1 = .
...

any advice on how best to handle this? custom rollup that I can pass into snowpack.config?

[Velua]

We're just following Node.js behavior here, but I do get your frustration. It's a technical limitation: CJS named exports are defined at runtime, so can't be statically analyzed.

However, I've heard rumors that Node.js is working on a pre-pass scanner that will do it's best to detect named exports automatically from Common.js packages. We'll wait until that's official before we try to implement ourselves, but that would mean that in a future version MOST common.js packages would support named exports by default.

Many popular packages are ESM now, so hopefully this won't be too hard to handle today. We can also look into adding a warning if we see you doing named imports from a common.js package.

Warning system would be good, or perhaps even something like a package.json checker to see how much dev time it's going to take to handle existing projects. Just tried shifting my production app to this to find all sorts of awesome errors. Would be good to have an idea of what packages exactly are getting in the way.

[ted-mccarthyfinch]

@cherihung did you figure out how to handle your issue? I'm running into the same issue with styled-components.

[cherihung]

@ted-mccarthyfinch no, I wasn't able to solve with changing snowpack config. since I had also control of the package I was consuming, I ended up tweaking it to publish both cjs and mjs components. and adding a "module" field to that package. I know it's not a real solution to the problem but it worked for my purpose at this time.

am still very much looking to implement a real solution though.