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

cannot use named import from tsc generate code converted by rollup-plugin-commonjs #556

Closed
zhangyuang opened this issue Aug 26, 2020 · 9 comments

Comments

@zhangyuang
Copy link

zhangyuang commented Aug 26, 2020

i hope rollup can generate bundle.js by export {} instead of export default

the following code was generated by tsc

// foo.js
const foo = 'foo'
exports.foo = foo

// index.js
"use strict";
function __export(m) {
    for (var p in m) if (!exports.hasOwnProperty(p)) exports[p] = m[p];
}
exports.__esModule = true;

__export(require("./foo"));

i want to convert index.js to esmodule by @rollup/plugin-commonjs ,but after generation, the module export mode is export default, but i want to use import { foo } from './bundle'

Expected Behavior

rollup can generate code like

const foo = 'foo'
export {
  foo
}

Actual Behavior

function getDefaultExportFromCjs (x) {
	return x && x.__esModule && Object.prototype.hasOwnProperty.call(x, 'default') ? x['default'] : x;
}

function createCommonjsModule(fn, basedir, module) {
	return module = {
	  path: basedir,
	  exports: {},
	  require: function (path, base) {
      return commonjsRequire(path, (base === undefined || base === null) ? module.path : base);
    }
	}, fn(module, module.exports), module.exports;
}

function commonjsRequire () {
	throw new Error('Dynamic requires are not currently supported by @rollup/plugin-commonjs');
}

const foo = 'foo';
var foo_2 = foo;

var foo_1 = {
	foo: foo_2
};

var js_test = createCommonjsModule(function (module, exports) {
function __export(m) {
    for (var p in m) if (!exports.hasOwnProperty(p)) exports[p] = m[p];
}
exports.__esModule = true;

__export(foo_1);
});

var index = /*@__PURE__*/getDefaultExportFromCjs(js_test);

export default index;

Additional Information

@shellscape
Copy link
Collaborator

shellscape commented Aug 26, 2020

Issue templates exist so that you can give us the info we need to triage bugs. As stated in the template, those fields are required. Add the missing fields, otherwise we won't triage your issue.

You appear to be asking how to do something, which is asking for support, and that's not something we offer here.

If this is an actual bug, please explain why it is a bug.

@zhangyuang
Copy link
Author

zhangyuang commented Aug 26, 2020

@shellscape my source code was compiled by tsc compile esmodule to cjs。now, i want to use rollup with plugin @rollup/plugin-commonjs transform code to esm。but the generated code use export default is not what i need。i need named export like export {}

@zhangyuang
Copy link
Author

zhangyuang commented Aug 27, 2020

@lukastaegert i'm sorry for disturb, i don't know whether describe the issue clearly

@zhangyuang
Copy link
Author

// foo.js
const foo = 'foo'
exports.foo = foo

the different between exports['foo'] = require('./foo')['foo'] and exports.foo = require('./foo').foo, the generate code by rollup-plugin-commonjs is export default xxx and export {xxx} 。but i want to use export {} when my source code is exports['foo'] = require('./foo')['foo']

@zhangyuang
Copy link
Author

zhangyuang commented Aug 27, 2020

it looks like cause by when node.computed is true, the expression exit

function flatten(node) {
  const parts = [];
  while (node.type === 'MemberExpression') {
    if (node.computed) return null;
    parts.unshift(node.property.name);
    // eslint-disable-next-line no-param-reassign
    node = node.object;
  }

  if (node.type !== 'Identifier') return null;

  const { name } = node;
  parts.unshift(name);

  return { name, keypath: parts.join('.') };
}

@lukastaegert
Copy link
Member

I am not sure I understand the issue fully but, if you want to control which exports are generated and specifically want named exports

Do not use CommonJS for entry points

Rollup is an ES module bundler, and when you use CommonJS, Rollup needs to convert it to ESM first. Here, information about exports can be lost.

  • The best solution would be to get tsc to generate ES modules as output.

  • If that is not possible for whatever reason, you can also just use a simple re-export module as entry point:

    // entry.js
    export {foo} from './index.js'

    This gives you full control over the exports.

Unfortunately due to the way tsc generates code here, it will be impossible for the foreseeable future to detect any named exports here.

@zhangyuang
Copy link
Author

zhangyuang commented Aug 27, 2020

@lukastaegert thanks for your reply, for this issue in vite, because some module like redux-dynamic-modules-core not generate esm module only cjs
image

@hansottowirtz
Copy link

hansottowirtz commented Aug 28, 2020

I was also wondering why this didn't work anymore in my libraries. It turned out I had to downgrade to ^12, or downgrade Typescript to ~3.8 use the esnext target in tsconfig.json. I think that in version 12, the Object.defineProperty style of assigning exports was being detected by this plugin, but in 13+ this apparently doesn't work anymore, and only the default export remains.

Minimal reproduction: https://github.com/hansottowirtz/rollup-export-repro

Example:

File src/a.js:

Object.defineProperty(exports, "a", { enumerable: true, get: function () { return 1; } });

rollup.config.js:

import commonjs from '@rollup/plugin-commonjs';

export default {
  input: './src/a.js',
  output: {
    file: `./dist/a.js`,
    format: 'cjs',
    exports: 'named'
  },
  plugins: [
    commonjs()
  ]
}

dist/a.js in version ^11:

'use strict';

Object.defineProperty(exports, '__esModule', { value: true });

function createCommonjsModule(fn, module) {
	return module = { exports: {} }, fn(module, module.exports), module.exports;
}

var a = createCommonjsModule(function (module, exports) {
Object.defineProperty(exports, "a", { enumerable: true, get: function () { return 1; } });
});
var a_1 = a.a;

exports.a = a_1;
exports.default = a;

Version ^12:

'use strict';

Object.defineProperty(exports, '__esModule', { value: true });

function createCommonjsModule(fn, basedir, module) {
	return module = {
	  path: basedir,
	  exports: {},
	  require: function (path, base) {
      return commonjsRequire(path, (base === undefined || base === null) ? module.path : base);
    }
	}, fn(module, module.exports), module.exports;
}

function commonjsRequire () {
	throw new Error('Dynamic requires are not currently supported by @rollup/plugin-commonjs');
}

var a = createCommonjsModule(function (module, exports) {
Object.defineProperty(exports, "a", { enumerable: true, get: function () { return 1; } });
});
var a_1 = a.a;

exports.a = a_1;
exports.default = a;

Version ^13, ^14, ^15:

'use strict';

Object.defineProperty(exports, '__esModule', { value: true });

function createCommonjsModule(fn, basedir, module) {
	return module = {
	  path: basedir,
	  exports: {},
	  require: function (path, base) {
      return commonjsRequire(path, (base === undefined || base === null) ? module.path : base);
    }
	}, fn(module, module.exports), module.exports;
}

function commonjsRequire () {
	throw new Error('Dynamic requires are not currently supported by @rollup/plugin-commonjs');
}

var a = createCommonjsModule(function (module, exports) {
Object.defineProperty(exports, "a", { enumerable: true, get: function () { return 1; } });
});

exports.default = a;

I'm not sure when Object.defineProperty is generated by tsc, but it seems like it happens during reexports. It was introduced in 3.9.2 by this PR: microsoft/TypeScript#35967

@stale
Copy link

stale bot commented Nov 7, 2020

Hey folks. This issue hasn't received any traction for 60 days, so we're going to close this for housekeeping. If this is still an ongoing issue, please do consider contributing a Pull Request to resolve it. Further discussion is always welcome even with the issue closed. If anything actionable is posted in the comments, we'll consider reopening it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants