eslint: Bug: `no-restricted-exports` fails on `export { default, ... } from ...`

Environment

Node version: v14.18.1 npm version: v8.3.0 Local ESLint version: v8.9.0 (Currently used) Global ESLint version: Not found Operating System: darwin 20.6.0

What parser are you using?

Default (Espree)

What did you do?

Minimal reproduction repo here: https://github.com/patcon/eslint-bug-reproduction-default-export

What did you expect to happen?

Expected npm run lint to pass, since

export { default, function2 } from 'bar.js';

is just an alternative format, recommended in Mozilla web MDN docs 😃

What actually happened?

Both forms of this default failed:

Screen Shot 2022-02-17 at 3 51 58 PM

Participation

  • I am willing to submit a pull request for this issue.

Additional comments

Re-ticketed from https://github.com/airbnb/javascript/issues/2500

Thanks a bunch! Open to pointers in the right direction, or just affirmation that this is a bug 🎉 🐛

About this issue

  • Original URL
  • State: closed
  • Created 2 years ago
  • Reactions: 6
  • Comments: 28 (22 by maintainers)

Most upvoted comments

Okay, here are some ideas.

a) Add a boolean option to re-allow 3 and 4. The option can be specified only if restrictedNamedExports contains "default". The following configuration would allow 1, 3, 4, and disallow 2, 5:

{
    "no-restricted-exports": ["error", {
        restrictedNamedExports: [
            "then",
            "default" // disallows 2, 3, 4, 5
        ],
        allowDefaultFrom: true // re-allows 3 and 4
    }]
}

b) Add an object option with boolean properties for each 1-5. The option can be specified only if restrictedNamedExports does not contain "default". The following configuration would allow 1, 3, 4, and disallow 2, 5:

{
    "no-restricted-exports": ["error", {
        restrictedNamedExports: [
            "then"
            // no "default" here
        ],
        restrictDefaultExports: {
            direct: false, // 1. export default foo; export default 42; export default function foo() {}
            named: true, // 2. export { foo as default };
            defaultFrom: false, // 3. export { default } from "mod"; export { default as default } from "mod";
            namedFrom: false, // 4. export { foo as default } from "mod";
            namespaceFrom: true // 5. export * as default from "mod"
        }
    }]
}

The b) is a bit more work but provides more than a), and I think it’s easier to understand. I’d vote for b).

@mdjermanovic i think that’s fine for export { default }, but this is export { default } from, which is a completely distinct form - meaning, it’s not as simple as those 3 options.

Hi @amareshsm are you still working on this?

@patcon I would like to work on this feature. let me know if anyone working on this.

This does look like a bug because, in this case, “default” is not a named export. @mdjermanovic do you agree?

@shirshendubhowmick There is an open PR for this - https://github.com/eslint/eslint/pull/16785. It will be fixed soon.

I agree that the second option is clearer.

@ljharb which of the following would you like to disallow?

  1. export default foo;
  2. export { foo as default };
  3. export { default } from "mod";
  4. export { foo as default } from "mod";
  5. export * as default from "mod";

The current behavior with restrictedNamedExports: ["default"] disallows 2, 3, 4, and 5.

Either would work for me, but i agree b is clearer and preferred.

The Airbnb config strongly pushes default export usage, so we’d want to disallow only number 3 (and i guess 5, but that’s less important). 2 and 4 are valid re-export forms of the preferred export method, 1.

Isn’t 3. export { default } from "mod"; the same as the example this issue is about? If the rule continues to disallow it, then it doesn’t seem this issue will be fixed.

An option seems like the fastest way to a solution. @mdjermanovic ?

Airbnb’s intention was to force default exports to use export default and NOT confusingly imply that “default” is Just Another Name, but export { default } from is the only way to do that, so we do NOT want to forbid that.

Using no-restricted-syntax, as always, isn’t a viable option for a shared config because it’s a) used for many things, and is hard to disable for just one of the items; and b) it doesn’t convey meaning/intent the way a dedicated rule does.

i think that’s fine for export { default }, but this is export { default } from, which is a completely distinct form - meaning, it’s not as simple as those 3 options.

Ah, ok! In that case, if there’s a need to disallow only specific ways to export "default" while allowing others, I think the best option is to configure no-restricted-syntax with the desired combination of selectors.

{
    "no-restricted-syntax": ["error", 

        // export default foo; 
        "ExportDefaultDeclaration",

        // export { foo as default };
        "ExportNamedDeclaration[source=null] > ExportSpecifier > :matches(Identifier[name='default'], Literal[value='default']).exported",

        // export { default } from "mod"; export { foo as default } from "mod";
        "ExportNamedDeclaration[source] > ExportSpecifier > :matches(Identifier[name='default'], Literal[value='default']).exported",

        // export * as default from "mod";
        "ExportAllDeclaration > :matches(Identifier[name='default'], Literal[value='default']).exported"      
    ]
}

It seems that we have agreed to fix this problem by implementing option b) from https://github.com/eslint/eslint/issues/15617#issuecomment-1048730114, so I marked this as accepted.

@patcon would you like to submit a PR for the new option?

ohhh right, sorry, i confused myself. We do want to allow option 3 😃 so looking again, we want to prohibit 2 (and maybe 5), but 1 is ideal, and 3 and 4 are the only ways to do that, so they’d also be allowed.

The Airbnb config strongly pushes default export usage, so we’d want to disallow only number 3 (and i guess 5, but that’s less important). 2 and 4 are valid re-export forms of the preferred export method, 1.

Could it be an option?

In particular, if the intent is not to disallow export { default }, then why is "default" in restrictedNamedExports[] in the configuration? What did you expect to be disallowed by restrictedNamedExports: ["default"]?

@mdjermanovic Though I suspect maybe the convo has moved past this question: that choice comes from airbnb’s rules, and I have no specific expectations (other than that something so highly used must know more than me 🙂 )

https://github.com/airbnb/javascript/blob/b4377fb03089dd7f08955242695860d47f9caab4/packages/eslint-config-airbnb-base/rules/es6.js#L65-L70