Chore: rename shadowed global

[UziTech]

Prerequisites checklist

• I have read the contributing guidelines.
• The team has reached consensus on the changes proposed in this pull request. If not, I understand that the evaluation process will begin with this pull request and won't be merged until the team has reached consensus.

What is the purpose of this pull request? (put an "X" next to an item)

[ ] Documentation update
[x] Bug fix (template)
[ ] New rule (template)
[ ] Changes an existing rule (template)
[ ] Add autofixing to a rule
[ ] Add a CLI option
[ ] Add something to the core
[ ] Other, please explain:

Tell us about your environment

• ESLint Version: 6.8.0
• Node Version: 12.13.0
• npm Version: 6.13.7

What did you do? Please include the actual source code causing the issue.

use rollup to include eslint in the browser

What did you expect to happen?

bundle eslint/lib/linter/linter.js

What actually happened? Please include the actual, raw output from ESLint.

rollup error:

[!] Error: Unexpected token
node_modules\eslint\lib\rules\no-obj-calls.js (50:33)
48:                 const traceMap = {};
49:
50:                 for (const global of nonCallableGlobals) {
                                     ^
51:                     traceMap[global] = {
52:                         [CALL]: true

What changes did you make? (Give an overview)

rename variable global to g

Is there anything you'd like reviewers to focus on?

[mdjermanovic]

I'm not against this change if it would help, but as far as I know global isn't a reserved word.

I believe this could be a bug in @rollup/plugin-commonjs as the code on which the parser throws seems to be the following:

for (const commonjsHelpers.commonjsGlobal of nonCallableGlobals) {
    traceMap[commonjsHelpers.commonjsGlobal] = {
        [CALL]: true
    }; 
}

It looks like the local global variable was transformed as if it was the Node's global. This doesn't happen if the plugin is used with commonjs({ ignoreGlobal: true }).

[ljharb]

global is very much not a reserved word.

[mdjermanovic]

I'm anyway 👍 to rename this variable because it's shadowing the global global, in order to avoid possible errors if the code gets changed in the future.

[UziTech]

commonjs({ ignoreGlobal: true }) did work to prevent the error in rollup, but I agree shadowing the global variable might have unintended effects in the future.

[mdjermanovic]

commonjs({ ignoreGlobal: true }) did work to prevent the error in rollup, but I agree shadowing the global variable might have unintended effects in the future.

Though, { ignoreGlobal: true } might break something else. It might be good to anyway open an issue at https://github.com/rollup/plugins since this does look like a bug in the plugin.

[kaicataldo]

Marking as accepted as I do think it's best not to shadow Node's global object. We should change the commit message. Maybe Chore: rename shadowed global?

[mdjermanovic]

LGTM, thanks!

[platinumazure]

Looks good, thanks!

[kaicataldo]

Thanks for contributing!