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

Possible false positive bug with destructured named exports #1

Closed
dburles opened this issue Nov 19, 2020 · 8 comments · Fixed by #2
Closed

Possible false positive bug with destructured named exports #1

dburles opened this issue Nov 19, 2020 · 8 comments · Fixed by #2
Labels
bug Something isn't working

Comments

@dburles
Copy link
Contributor

dburles commented Nov 19, 2020

Reproduction: https://github.com/dburles/find-unused-exports-issue

I found that exporting named exports in this way results in a false positive with no exports named:

index.js
  
1 unused export in 1 module.
@dburles dburles changed the title Possible false positive bug with named exports Possible false positive bug with destructured named exports Nov 19, 2020
@jaydenseric jaydenseric added the bug Something isn't working label Nov 19, 2020
@jaydenseric
Copy link
Owner

Interesting, maybe you could take a stab at a PR?

@jaydenseric
Copy link
Owner

Probably a destructured variable export hasn't been accounted for near here:

case 'VariableDeclaration':
// E.g. `export const a = true`
// ^^^^^^^^^^^^^^
// E.g. `export var a, b, c = true`
// ^^^^^^^^^^^^^^^^^^
path.node.declaration.declarations.forEach(({ id: { name } }) => {
analysis.exports.add(name);
});

@jaydenseric
Copy link
Owner

jaydenseric commented Nov 19, 2020

Also, what about array destructuring? E.g. something like:

export const [a] = [true];

@jaydenseric
Copy link
Owner

Also, need to account for this:

export const { a, b: c } = { a: true, b: true };

@jaydenseric
Copy link
Owner

jaydenseric commented Nov 19, 2020

What about this:

export const [, a] = [true, true];

Edit: So I tested the above in Node.js, using:

import * as a from './a.mjs';

console.log(a);

And the result was [Module] { a: true }, so it does seem to be valid and we need to account for it.

@jaydenseric
Copy link
Owner

Holy cow! Just released this needs to be a recursive operation as the destructuring can be arbitrarily deep/complex:

export const { k, l: { m: [n]} } = { k: true, l: { m: [true] } };

In that case the named exports are k and n.

@jaydenseric
Copy link
Owner

Also, spread syntax:

export const { a, ...b } = { a: true, b: true, c: true };
export const [, ...c] = [1, 2];

@jaydenseric
Copy link
Owner

The fix has been published in v1.1.1 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants