Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

getCjsExportFromNamespace messes up default exports #491

Closed
Veetaha opened this issue Jul 8, 2020 · 3 comments · Fixed by #507
Closed

getCjsExportFromNamespace messes up default exports #491

Veetaha opened this issue Jul 8, 2020 · 3 comments · Fixed by #507

Comments

@Veetaha
Copy link

Veetaha commented Jul 8, 2020

  • Rollup Plugin Name: @rollup/plugin-commonjs
  • Rollup Plugin Version: 13.0.0
  • Rollup Version: 2.18.1
  • Operating System (or Browser): Vscode extension host (I guess it is bare nodejs process)
  • Node Version: 12.8.1

How Do We Reproduce?

Link to REPL

Use the following rollup.config.js:

import resolve from '@rollup/plugin-node-resolve';
import commonjs from '@rollup/plugin-commonjs';
import nodeBuiltins from 'builtin-modules';

export default {
    input: 'out/src/main.js',
    plugins: [
        resolve({
            preferBuiltins: true
        }),
        commonjs()
    ],
    external: [...nodeBuiltins, 'vscode'],
    output: {
        file: './out/src/main.js',
        format: 'cjs'
    }
};

Add "node-fetch": "^2.6.0" to package.json
Write the following code (in reality it is generated by tsc as the first step before invoking rollup):

const fetch = require("node-fetch");
console.log(typeof fetch.default);

When run in node REPL this works fine:

Output
$ node
Welcome to Node.js v14.4.0.
Type ".help" for more information.
> const fetch = require("node-fetch");
undefined
> console.log(typeof fetch.default)
function

Expected Behavior

When packaged with rollup it must work the same as in node REPL (it must not crash in our production!)

Actual Behavior

It outputs undefined.
The reason is that the bundle contains the following:

function getCjsExportFromNamespace (n) {
	return n && n['default'] || n;  // <--- this is the culprit
}

var lib = /*#__PURE__*/Object.freeze({
	__proto__: null,
	'default': fetch,
	Headers: Headers,
	Request: Request,
	Response: Response,
	FetchError: FetchError
});

var fetch = getCjsExportFromNamespace(lib);

console.log(typeof fetch.default);

cc @lukastaegert
related: rust-lang/rust-analyzer#5257 (comment)

@lukastaegert
Copy link
Member

Looks like this one should be added to #481

@shellscape
Copy link
Collaborator

Thanks for opening an issue. Citing the issue template:

Issues without minimal reproductions will be closed! Please provide one by:

  1. Using the REPL.it plugin reproduction template at https://repl.it/@rollup/rollup-plugin-repro
  2. Provide a minimal repository link (Read https://git.io/fNzHA for instructions).
    These may take more time to triage than the other options.
  3. Using the REPL at https://rollupjs.org/repl/

Please add a reproduction and we'll be happy to triage further.

@Veetaha
Copy link
Author

Veetaha commented Jul 8, 2020

@shellscape here is the reproduction: https://repl.it/join/vhpbczoh-veetaha
try running
node ./output/bundle.js and node ./input.js to see the difference

bors bot added a commit to rust-lang/rust-analyzer that referenced this issue Jul 8, 2020
5262: Workaround rollup messing up default imports r=matklad a=Veetaha

Tackles #5257 (comment)
Related: rollup/plugins#491

Co-authored-by: Veetaha <veetaha2@gmail.com>
@shellscape shellscape changed the title getCjsExportFromNamespace messes up default exports (how to disable it?) getCjsExportFromNamespace messes up default exports Jul 9, 2020
matklad pushed a commit to matklad/vscode-rust that referenced this issue Jul 13, 2020
5262: Workaround rollup messing up default imports r=matklad a=Veetaha

Tackles rust-lang/rust-analyzer#5257 (comment)
Related: rollup/plugins#491

Co-authored-by: Veetaha <veetaha2@gmail.com>
bors bot added a commit to rust-lang/rust-analyzer that referenced this issue Dec 8, 2020
6759: Remove workaround & fix fetch$1 is not a function r=lnicola a=kafji

Remove workaround for rollup/plugins#491
because it's fixed in 15.0
https://github.com/rollup/plugins/blob/master/packages/commonjs/CHANGELOG.md#v1500.

Also fix fetch$1 is not a function error
#6757.

Co-authored-by: Kafji <k@kafji.net>
Matthias-Fauconneau pushed a commit to Matthias-Fauconneau/rust-analyzer that referenced this issue Feb 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants