eslint-plugin-react icon indicating copy to clipboard operation
eslint-plugin-react copied to clipboard

[New] : Avoid unsafe global window use

Open jzabala opened this issue 5 years ago • 4 comments

This rule will prevent the unsafe use on window resulting from rendering React apps in environments where the global is not available.

I'll be grateful for getting all your comments and suggestions before summiting the final version.

Tested Conditions

  • [x] used in a guard
  • [x] used in componentDidMount, componentDidUpdate and componentWillUnmount
  • [x] used in useEffect, useLayoutEffect and useCallback
  • [x] used in an event handler

Considerations

The issue talks about disabling the use of various globals variables aside from window. I think an easier solution would be to use the rule no-restricted-globals to force the use of window in the variables that we want.

Another solution would be to have the option of setting what globals will be disabled directly in this plugin with window as default. And to change the name of the rule to something like: no-unsafe-globals-use.

Let me know your opinion about this or if you have other ideas

Closes #2057

jzabala avatar Jun 30 '20 17:06 jzabala

This looks like a good start :-)

Thanks for your initial review @ljharb.

I think it is ready now 🙂

jzabala avatar Jul 06 '20 17:07 jzabala

i'm interested in this rule or something like it getting published - anything that needs to be done to help it along?

osdiab avatar Oct 24 '20 07:10 osdiab

FYI currently this errors incorrectly:

typeof window === "undefined" ? undefined : window.location.href

Alternatively,

if (typeof window === "undefined") {
  return;
}
console.log(window.location.href);

Also shadowing window still causes errors, like so:

function getWindow() {
  return typeof window !== "undefined" ? window : undefined;
}

// another file
const window = getWindow(); // this line errors but pretty sure it's actually fine
console.log(window?.location.href); // this errors too

though just giving it a different name works fine.

osdiab avatar Oct 24 '20 08:10 osdiab

also would probably make sense to check for document, but yeah maybe that makes more sense for no-restricted-globals

osdiab avatar Oct 24 '20 09:10 osdiab