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

Mangled generated code mixing ExportAllDeclaration and external source #2489

Closed
usergenic opened this issue Oct 2, 2018 · 4 comments
Closed

Comments

@usergenic
Copy link

  • Rollup Version: 0.64.0
  • Operating System (or Browser):
  • Node Version: 10.9.0

How Do We Reproduce?

Here's a fairly simple reproduction, call it repro.js

var { rollup } = require('rollup');

const files = {
  'file:///a.js': `import * as $b from "file:///b.js"; export { $b };`,
  'file:///b.js': `export * from 'file:///c.js';`,
  'file:///c.js': `export const c = '🤡';`
};

rollup({
  input: 'file:///a.js',
  external: ['file:///c.js'],
  output: { format: 'esm' },
  plugins: [{ name: "somePlugin", resolveId: (id) => id, load: (id) => files[id] }]
}).then((r) => r.generate({ format: 'es' }).then((r) => console.log(r.code)));

Run the repro:

npm install rollup@0.64.0
node repro.js

Expected Behavior

I'd expect something along the lines of the following, but would be happy with anything that achieves

import * as c from 'file:///c.js';
var $b = {
  c: c
};
export { $b };

Actual Behavior

Generates the following mess.

import 'file:///c.js';



var b = /*#__PURE__*/Object.freeze({
  *file:///c.js: *
});

export { b as $b };
@lukastaegert
Copy link
Member

lukastaegert commented Oct 7, 2018

This is indeed a tricky one. Your expected result is wrong IMO since if you do

import * as c from 'file:///c.js';

with file:///c.js as given in your example, then the namespace variable c would have the form {c: 🤡}. Thus in your example, $b$ should have this form as well while in your proposed solution, it would have the form {c: {c: 🤡}}.

A valid solution would be

import * as c from 'file:///c.js';
export { c as $b };

However, this is no longer possible if file:///b.js adds other imports, e.g.

// b.js
export * from 'file:///c.js';
export const dontKnowWhatToDoWithThis = 4;

My feeling is that we should try to use the first solution if possible but throw an error in the second case until someone figures out a nice solution that scales well.

@lukastaegert
Copy link
Member

Interestingly enough, rollup 0.66 refuses to bundle this code with an internal error...

@lukastaegert
Copy link
Member

Not really a fix but a better error message: #2499

@lukastaegert
Copy link
Member

Closing this in favour of #2165 as with the correct error message, this missing feature is all that remains.

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

2 participants