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

import/export - Multiple exports triggered when duplication isn't relevant #1704

Closed
tomprats opened this issue Mar 29, 2020 · 18 comments
Closed

Comments

@tomprats
Copy link
Contributor

tomprats commented Mar 29, 2020

I get the error Multiple exports of name 'List' from both lines of components/pages/index.js in following example:

// components/pages/index.js
export * as Messages from "./messages";
export * as Users from "./users";

// components/pages/messages/index.js
export List from "./list";
export Show from "./show";

// components/pages/users/index.js
export Edit from "./edit";
export List from "./list";

// components/pages/messages/list.js => React Component
// components/pages/messages/show.js => React Component
// components/pages/users/edit.js => React Component
// components/pages/users/list.js => React Component

I understand that if I get it once, I'll get it twice, but I don't think it should be happening at all. The code itself works when I access my components (Pages.Users.List) leading me to believe this is a bug in the rule. I also made sure to update to the latest version (2.20.1) before submitting this

@ljharb
Copy link
Member

ljharb commented Mar 29, 2020

When you say "the code works", do you mean, in native browser or node ESM, or in babel/typescript?

@ljharb
Copy link
Member

ljharb commented Mar 29, 2020

In other words, I believe the spec requires this to be a syntax error, because you have two exports that conflict. Babel might paper over this by silently choosing the last one, but that doesn't mean it's valid.

@tomprats
Copy link
Contributor Author

Oh yeah, my bad. I'm using babel and I think the specific relevant plugin is plugin-proposal-export-namespace-from which looks like it's stage 4.

I think that might change the context of how it works, but do they conflict? With that plugin I can access both Pages.Messages.List and Pages.Users.List. I thought it was fine because it'd immediately assign the * export to the namespace (Messages or Users) so their contents wouldn't pollute anything

@tomprats
Copy link
Contributor Author

Also in case it's not clear, the commented out portions above are meant to denote individual files. They're not all in one file

@golopot
Copy link
Contributor

golopot commented Mar 30, 2020

When you say "the code works", do you mean, in native browser or node ESM, or in babel/typescript?

The example works without syntax error in node ESM. The index.js module has two exports, Messages and Users. But our plugin reports Multiple exports of name 'List'. Something is wrong here.

@CatsMiaow
Copy link

https://www.typescriptlang.org/docs/handbook/release-notes/typescript-3-8.html#export-star-as-namespace-syntax
The export * as was added in TypeScript v3.8

Playground

// ts
export * as Messages from "./messages";
export * as Users from "./users";
// js
exports.Messages = require("./messages");
exports.Users = require("./users");

Using export * as does not conflict even if the property names are the same.
No Multiple exports of name 'List' errors should occur.

@ljharb
Copy link
Member

ljharb commented Apr 1, 2020

Ah, very true - export * as X can't conflict with anything but another X.

This definitely seems like a bug.

@thecotne
Copy link

thecotne commented May 9, 2020

@ljharb can you remove typescript label since Proposal: export ns from is part of standard js tc39/ecma262#1174

@ljharb ljharb removed the typescript label May 10, 2020
@danrivett
Copy link

Also ran into this issue with the following code:

export * as WorkerPhoneCreated from './phone/created';
export * as WorkerPhoneUpdated from './phone/updated';

Both the following imported files contain an EventType export, and should be referenced as WorkerPhoneCreated.EventType and WorkerPhoneUpdated.EventType respectively and so I think this "multiple exports" error is a false positive if I've understood correctly.

@ljharb
Copy link
Member

ljharb commented Jun 21, 2020

I agree; that should be addressed.

@lifeiscontent
Copy link

@ljharb any updates on this? I just ran into this recently.

@ljharb
Copy link
Member

ljharb commented Aug 8, 2020

@lifeiscontent no; nobody's sent a PR yet and I haven't had the time. One would be welcome :-)

@tomprats
Copy link
Contributor Author

I started to attempt a fix today and ran into an issue with the current ecmaVersion being too old for the syntax. But after I updated that, I found out the issue has already been fixed! Potentially here. Below is a test specifically for it that I can add, but I don't know if there are any further repercussions for updating the ecmaVersion

--- a/tests/src/rules/export.js
+++ b/tests/src/rules/export.js
@@ -35,6 +35,12 @@ ruleTester.run('export', rule, {
         export { A, B };
       `,
     }),
+    test({
+      code: `
+        export * as A from './named-export-collision/a';
+        export * as B from './named-export-collision/b';
+      `,
+    }),
   ],

   invalid: [
diff --git a/tests/src/utils.js b/tests/src/utils.js
index 0f45e7b..0892482 100644
--- a/tests/src/utils.js
+++ b/tests/src/utils.js
@@ -37,7 +37,7 @@ export function test(t) {
   }, t, {
     parserOptions: Object.assign({
       sourceType: 'module',
-      ecmaVersion: 9,
+      ecmaVersion: 11,
     }, t.parserOptions),
   })
 }

Long story short, this is fixed on version 2.22.0. Thanks @ljharb!

@ljharb
Copy link
Member

ljharb commented Aug 21, 2020

No problem; the ecmaVersion can be changed on a per-test basis :-)

I've adapted your diff into a commit, thanks!

@ftaiolivista
Copy link

Still getting this error on v 2.23.4

// D.js 
const f = () => 'D::f'
export { f }

// E.js
export * from './D.js'
const f = () => 'E::f'
export { f }

-> import/export: Multiple exports of name 'f'.

@thecotne
Copy link

thecotne commented Jul 23, 2021

@ftaiolivista that is legit error. not a bug in eslint-plugin-import

@ftaiolivista
Copy link

I think the export overloading is ammisible by ECMASCRIPT (see this answer https://stackoverflow.com/questions/66533367/import-export-name-collision-resolution )

Maybe eslint could give an optionable error that can ben disabled by config?

@ljharb
Copy link
Member

ljharb commented Jul 26, 2021

@ftaiolivista i agree; could you file a new issue for that?

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

No branches or pull requests

8 participants