fix(dev): preserve named exports when optimizing cjs deps

[csr632]

Fix: #720 #815

The problem

Rollup can't handle cjs entry very well: rollup/plugins#572. When entry module is a commonjs module, the rollup output bundle only have one default export with everything in it. That's why when vite is optimizing cjs deps, users can only import default from it.

But most users are used to write named import to import things from cjs deps. For example import { useState } from 'react'. Before this fix, this line will cause error in vite serve: Uncaught SyntaxError: The requested module '/@modules/react.js' does not provide an export named 'useState'.

This fix

Auto static analysis of named export

This pr implement a rollup plugin to detect all possible named export from cjs entries in optimizing step (using cjs-module-lexer). With this fix, users can finally use named import to import things from commonjs dependencies:

import React, { useState, createContext } from 'react'

Most users are used to write this kind of import when using webpack.

With this feature, most cjs dependencies like react and react-dom will work out of box. vite-plugin-react no longer need to alias react into @pika/react.

Manually specify the named exports of commonjs dependencies

The static analysis have some limitations. So we also support users to "manually specify the named exports of commonjs dependencies" in vite config. For example:

optimizeDeps: {
    cjsExports: {
      'prop-types': ['oneOfType']
    }
}

With this config, users can writeimport { oneOfType } from 'prop-types' in their code.

[csr632]

@underfin @antfu @yyx990803
Could you please review this?

[antfu]

Looking great! Just wondering if it's better to be done on the rollup side? Is there any obstacles?

[csr632]

The main idea of this fix is in this rollup plugin: src/node/optimizer/pluginCjsEntry.ts. In the options hook I replace cjs entry with a proxy entry module which re-export all named export from the original cjs entry.

@lukastaegert Do you think this fix can be done in the official @rollup/plugin-commonjs?

[lukastaegert]

Without looking deeper into this plugin: It depends on which files this plugins is applied to. If it is only applied to entry points then it would definitely be a valuable addition to the commonjs plugin. The reason why it should not apply to non-entry points is that here, Rollup already has a better logic in place that is capable of retaining live-bindings to some extent. But from the description it sounds like a very reasonable approach. Will need to have a deeper look the next days.

[csr632]

If it is only applied to entry points then it would definitely be a valuable addition to the commonjs plugin.

Yes it only apply to entry points.

[underfin]

It is great work.But I think the #720 (comment) is better because the user don't apply cjsExports options for some packages which has non-dected named exports.

[csr632]

It is great work.But I think the #720 (comment) is better because the user don't apply cjsExports options for some packages which has non-dected named exports.

Agreed. If we do the transformation in the side of "importer", we would have the information that what user actually import.

I will try this approach in a seperate PR. So that we can still discuss how can we improve @rollup/plugin-commonjs to make it support enrty with named export here.

[csr632]

I have created #837 which is implemented with the better approach.
@underfin @antfu

[csr632]

Although in #837 we take the "consumer side transformation" approach to fix the vite bug, it is just "hiding" the rollup bug.

The rollup plugin in this pr is still worth to be considered, to fix the fundamental bug in @rollup/plugin-commonjs.

[yyx990803]

Closing in favor of #837