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

eslint complains about `global-require` even if the require usage is scoped #4812

Closed
satazor opened this Issue Dec 26, 2015 · 10 comments

Comments

Projects
None yet
7 participants
@satazor
Copy link
Contributor

satazor commented Dec 26, 2015

const someFunc = require('./someFunc');

someFunc(function (require) {
    require('bananas', function () {  // eslint complains that I should put this require at the top

   });
});

Obviously one could rename require to req to make this issue go away, but I think eslint should be smarter and not complain in the situation exemplified above.

Thoughts?

Version: 1.10.3

@eslintbot

This comment has been minimized.

Copy link

eslintbot commented Dec 26, 2015

@satazor Thanks for the issue! If you're reporting a bug, please be sure to include:

  1. The version of ESLint you are using (run eslint -v)
  2. What you did (the source code and ESLint configuration)
  3. The actual ESLint output complete with numbers
  4. What you expected to happen instead

Requesting a new rule? Please see Proposing a New Rule for instructions.

@eslintbot eslintbot added the triage label Dec 26, 2015

@michaelficarra

This comment has been minimized.

Copy link
Member

michaelficarra commented Dec 26, 2015

Agreed. We should be able to easily make sure reported require calls are referencing the global require function.

@nzakas

This comment has been minimized.

Copy link
Member

nzakas commented Dec 26, 2015

This is working as intended. The goal of the rule is to mimic how ES6 module imports work.

Your best bet in this situation is to disable the rule around that usage.

@nzakas nzakas closed this Dec 26, 2015

@michaelficarra

This comment has been minimized.

Copy link
Member

michaelficarra commented Dec 26, 2015

@nzakas You may want to re-read the issue. The rule is flagging calls to any function named require.

@satazor

This comment has been minimized.

Copy link
Contributor Author

satazor commented Dec 26, 2015

@nzakas I don't think it is intended because, in the example, the require is a function argument, not a global require. The require argument in that function could be any arbitrary function, not related to CommonJS's require whatsoever.

@nzakas nzakas reopened this Jan 4, 2016

@nzakas

This comment has been minimized.

Copy link
Member

nzakas commented Jan 4, 2016

Thanks @michaelficarra @btmills and @ilyavolodin for pointing out that I misunderstood this issue. I was too quick to close it.

@satazor you're right, we should be able to tell the difference between a reference to the global require and a different one.

@nzakas nzakas added bug rule accepted and removed triage labels Jan 4, 2016

@platinumazure

This comment has been minimized.

Copy link
Member

platinumazure commented Jan 4, 2016

I could use some experience with escope. Anyone mind if I take this on this week?

@nzakas

This comment has been minimized.

Copy link
Member

nzakas commented Jan 5, 2016

Go for it

@nzakas nzakas closed this in c4c4139 Jan 16, 2016

nzakas added a commit that referenced this issue Jan 16, 2016

Merge pull request #4872 from platinumazure/scoped-global-require
Fix: global-require no longer warns if require is shadowed (fixes #4812)
@Willibaur

This comment has been minimized.

Copy link

Willibaur commented Apr 21, 2017

Hi all, I have a similar situation than @satazor here is my piece of code where ESlin is complaining

  const moment = require('moment');
  function formatDate(date) {
    return moment(date).format('MMMM Do YYYY');
  }

According to the DOCS that is completely valid code,

// requiring a module and using it in a function is ok
var fs = require('fs');
function readFile(filename, callback) {
    fs.readFile(filename, callback)
}

So pls let me know what am I doing wrong here.
Thanks in advance.

@btmills

This comment has been minimized.

Copy link
Member

btmills commented Apr 21, 2017

@Willibaur if you haven't figured out what was going on yet, can you open a new issue and @-mention me? The information in our bug report template will help us track down what's happening. For what it's worth, the code looks correct to me, and the demo doesn't complain about it.

@eslint eslint bot locked and limited conversation to collaborators Feb 6, 2018

@eslint eslint bot added the archived due to age label Feb 6, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.