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

[patcon]

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:

Participation

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

Additional comments

Re-ticketed from airbnb/javascript#2500

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

[nzakas]

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

[mdjermanovic]

This works as intended, per the documentation for this rule on default exports:

https://eslint.org/docs/rules/no-restricted-exports#default-exports

An example of incorrect code in that section is basically the same as the example from the original post in regard to default:

/*eslint no-restricted-exports: ["error", { "restrictedNamedExports": ["default"] }]*/

export { default } from "some_module";

Adding "default" to restrictedNamedExports[] means that you want to disallow exporting "default" in named export declarations.

[patcon]

Thanks @mdjermanovic and @nzakas! Sorry, I am admittedly quite new to this export from syntax (and even a bit rookie with ES6), but am I trying to reconcile the existing eslint doc (that you linked) with the MDN docs...

The "export from" syntax allows the as token to be omitted; however this will mean the default item cannot be imported as a named import:

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

Thoughts @ljharb? (as you initially suggested this was an upstream fix in airbnb/javascript#2500 (comment))

EDIT: Ah seeing both you and ljharb were involved in the initial rule this came in on, so I'll just listen:

• initial issue: New rule: no-restricted-exports #10428
• specific comment about this decision in PR: New: Add no-restricted-exports rule (fixes #10428) #12546

[ljharb]

The original intention for the rule didn’t consider this, because default really isn’t a named export, conceptually - it’s just unfortunately implemented and specified as one.

I think that if “default” is restricted, then it should apply to default exports whether named or not. If it doesn’t apply to both, it shouldn’t apply to either.

[mdjermanovic]

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 :)

@patcon can you please clarify the expected behavior of this rule with your configuration?

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]

I think that if “default” is restricted, then it should apply to default exports whether named or not. If it doesn’t apply to both, it shouldn’t apply to either.

We had the following options on how should the rule behave when it's configured with restrictedNamedExports: ["default"]:

1. Throw configuration error.
2. Disallow export { default }.
3. Disallow export { default } and export default.

This was discussed in #12546, and we eventually decided on 2. as that seemed the most flexible. The behavior is, I believe, well-documented in the Default Exports section.

We didn't implement an option to disallow export default, which was mentioned in the PR, but it can be easily done with no-restricted-syntax, so all combinations are covered.

• Disallow export default:

      "rules": {
           "no-restricted-syntax": ["error", "ExportDefaultDeclaration"]
       } 

• Disallow export { default }:

      "rules": {
           "no-restricted-exports": ["error", { "restrictedNamedExports": ["default"] }]
       } 

• Disallow export default and export { default }:

      "rules": {
           "no-restricted-exports": ["error", { "restrictedNamedExports": ["default"] }],
           "no-restricted-syntax": ["error", "ExportDefaultDeclaration"]
       } 

[ljharb]

@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.

[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.

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"      
    ]
}

[patcon]

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

[ljharb]

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.

[mdjermanovic]

Since the current behavior is intentional and the rule works as documented, changing this would be a breaking change. Some users might be relying on this rule in combination with "no-restricted-syntax": ["error", "ExportDefaultDeclaration"] to entirely disallow default exports in their codebase.

If we are going to make a breaking change, I think we should either include export default or throw a configuration error, rather than replacing the current subset of ways to export "default" with another subset.

[ljharb]

Could it be an option?

[nzakas]

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

[mdjermanovic]

@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.

[ljharb]

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.

[mdjermanovic]

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.

[ljharb]

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.

[mdjermanovic]

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).

[ljharb]

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

[nzakas]

I agree that the second option is clearer.

[mdjermanovic]

It seems that we have agreed to fix this problem by implementing option b) from #15617 (comment), so I marked this as accepted.

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

[patcon]

@mdjermanovic I'd love to take a swing at it :) Sorry for not stepping up earlier -- the conversation felt a little beyond my ability to weigh in, but happy to try to make my first contrib here 🙂

Any pointers to specific areas of docs/codebase related to this, or that might be similar? (aside from things I'd learn in general docs, which I'm sure I can navigate on my own without taking your time)

[patcon]

(and pls feel welcome to assign me so that it stays in my dashboard)

[nzakas]

The rule docs page is a good place to start:
https://eslint.org/docs/rules/no-restricted-exports#resources

[amareshsm]

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

[upeguiborja]

Hi @amareshsm are you still working on this?

[shirshendubhowmick]

Any update on this ? This issue is still there.

[snitin315]

@shirshendubhowmick There is an open PR for this - #16785. It will be fixed soon.