Rule Change: no-implicit-globals should not report exported

[kkmuffme]

What rule do you want to change?

no-implicit-globals

What change to do you want to make?

Implement suggestions

How do you think the change should be implemented?

A new default behavior

Example code

/* exported testFunc */

/**
 * @returns {int}
 */
function testFunc() {
    "use strict";

    return 1 + 2;
}

What does the rule currently do for this code?

When using with lexicalBindings /*eslint no-implicit-globals: ["error", {"lexicalBindings": true}]*/ it gives an error

What will the rule do after it's changed?

Not report an error, if the variable/function is "exported"

Additional comments

This was already requested previously but closed due to age #10066

Essentially it should have the same behavior as rule no-unused-vars, where exported variables/functions don't count as unused

[mdjermanovic]

Not report an error, if the variable/function is "exported"

Makes sense to me 👍

I think the main purpose of this rule is to prevent accidental creation of global variables. Declaring /* exported testFunc */ indicates the intention to create a global testFunc, and would be an alternative to the style window.testFunc = function (){} that this rule currently enforces.

[nzakas]

Makes sense to me. Should this be a bug rather than an enhancement?

[sosukesuzuki]

Can I work on this?

[kkmuffme]

Yes please :-)

[mdjermanovic]

Should this be a bug rather than an enhancement?

I think this was an oversight in the design because the /* exported */ comments feature already existed when this rule was created. Currently allowed way is to add a /* global testFunc: writable */ comment (here's a playground example), which seems wrong because the variable doesn't exist before running the script.

[nzakas]

Just to clarify: you’re saying this is an enhancement because the rule intentionally didn’t consider this behavior?

[kkmuffme]

No, I think he means it's a bug, because it was forgotten to consider exported when this rule was created

[mdjermanovic]

Sorry for the confusion. Supporting /* global testFunc: writable */ as a way to allow global function testFunc by using eslint directives may imply that the other directives such as /* exported testFunc */ were considered and intentionally not supported, but it's hard to tell because exported was not explicitly mentioned anywhere in the original issue and the PR.

Either way, I think we can treat this as a bug fix.

However, if this requires adding a new property on Variable objects (#16343 (comment)), then it seems more appropriate to mark the change as a feat, as it will include an addition to the core.

[nzakas]

However, if this requires adding a new property on Variable objects (#16343 (comment)), then it seems more appropriate to mark the change as a feat, as it will include an addition to the core.

Sounds good.