feat(confusing-browser-globals): Add relevant messaging to no-restricted-globals eslint rule

[yanneves]

BREAKING CHANGE: export from confusing-browser-globals package changed from String[] to Object[] - no change required if used in eslint no-restricted-globals config

I came across unclear messaging in eslint-config-airbnb today and traced it to this package, which is included in eslint-config-airbnb-base.

If you have code like:

const SomeComponent = () => {
  useEffect(() => {
    const queryString = location.search.substr(1)
    // do something with query string
  }, [])

  // ...
}

Using this package in eslint no-restricted-globals rules will result in the error Unexpected use of 'location'. no-restricted-globals

Which may communicate "don't use location at all" rather than the intended "prefer window.location or a local parameter to avoid unintended effects"

So I've updated this package with clearer messaging, like in this case Unexpected use of 'location'. Use local parameter or window.location instead no-restricted-globals

And in doing so realised we don't want to recommend using window.<property> everywhere, for example the following code:

const handleEvent = () => {
  doSomethingWith(event.target.value)
}

We'd catch this and suggest using the (missing) local value and not window.event.

So I extended the list with a suggestion between window, document, WorkerGlobalScope*, or none to determine the correct messaging, and added tests covering each scenario.

Since the output of this package changes, it would break anywhere that strictly expects an array of strings to be exported (i.e. not being used in eslint config). It should release as a breaking change and new major version.

* WorkerGlobalScope applies to self included in this list, which made most sense to me but could be removed entirely from the list considering https://developer.mozilla.org/en-US/docs/Web/API/Window/self

[stale]

This pull request has been automatically marked as stale because it has not had any recent activity. It will be closed in 5 days if no further activity occurs.

[yanneves]

Still relevant if anyone's available to review

[JoshuaCWebDeveloper]

This PR would be useful! It has my 👍 .

self is a property on the WorkerGlobalScope instance (it isn't static), so WorkerGlobalScope.self actually returns undefined.

A couple of alternatives to using self are: globalThis (older browsers not supported) and this (definitely not recommended). Personally, I believe self is better than either of these.