[ESLint] Feedback for 'exhaustive-deps' lint rule

[gaearon]

Common Answers

💡💡💡💡💡💡💡💡💡💡💡💡💡💡💡💡💡💡💡💡💡💡

We analyzed the comments on this post to provide some guidance: https://github.com/facebook/react/issues/14920#issuecomment-471070149.

💡💡💡💡💡💡💡💡💡💡💡💡💡💡💡💡💡💡💡💡💡💡

---

What is this

This is a new ESLint rule that verifies the list of dependencies for Hooks like useEffect and similar, protecting against the stale closure pitfalls. For most cases it has an autofix. We'll add more documentation over the next weeks.

Installation

yarn add eslint-plugin-react-hooks@next
# or
npm install eslint-plugin-react-hooks@next

ESLint config:

{
  "plugins": ["react-hooks"],
  // ...
  "rules": {
    "react-hooks/rules-of-hooks": 'error',
    "react-hooks/exhaustive-deps": 'warn' // <--- THIS IS THE NEW RULE
  }
}

Simple test case to verify the rule works:

function Foo(props) {
  useEffect(() => {
    console.log(props.name);
  }, []); // <-- should error and offer autofix to [props.name]
}

The lint rule complains but my code is fine!

If this new react-hooks/exhaustive-deps lint rule fires for you but you think your code is correct, please post in this issue.

---

BEFORE YOU POST A COMMENT

Please include these three things:

1. A CodeSandbox demonstrating a minimal code example that still expresses your intent (not "foo bar" but actual UI pattern you're implementing).
2. An explanation of the steps a user does and what you expect to see on the screen.
3. An explanation of the intended API of your Hook/component.

But my case is simple, I don't want to include those things!

It might be simple to you — but it’s not at all simple to us. If your comment doesn't include either of them (e.g. no CodeSandbox link), we will hide your comment because it’s very hard to track the discussion otherwise. Thank you for respecting everyone’s time by including them.

The end goal of this thread is to find common scenarios and transform them into better docs and warnings. This can only happen when enough details are available. Drive-by comments with incomplete code snippets significantly drive down the quality of the discussion — to the point that it's not worth it.

[billyjanitsch]

This example has a reply: https://github.com/facebook/react/issues/14920#issuecomment-466145690

CodeSandbox

This is an uncontrolled Checkbox component which takes a defaultIndeterminate prop to set the indeterminate status on initial render (which can only be done in JS using a ref because there's no indeterminate element attribute). This prop is intended to behave like defaultValue, where its value is used only on initial render.

The rule complains that defaultIndeterminate is missing from the dependency array, but adding it would cause the component to incorrectly overwrite the uncontrolled state if a new value is passed. The dependency array can't be removed entirely because it would cause the indeterminate status to be fully controlled by the prop.

I don't see any way of distinguishing between this and the type of case that the rule is meant to catch, but it would be great if the final rule documentation could include a suggested workaround. 🙂

[gaearon]

Re: https://github.com/facebook/react/issues/14920#issuecomment-466144466

@billyjanitsch Would this work instead? https://codesandbox.io/s/jpx1pmy7ry

I added useState for indeterminate which gets initialized to defaultIndeterminate. The effect then accepts [indeterminate] as an argument. You currently don't change it — but if you did it later, I guess that would work too? So the code anticipates future possible use case a bit better.

[dan-lee]

So I got this following (edge?) case where I pass some html and use it with dangerouslySetInnerHtml to update my component (some editorial content). I am not using the html prop but the ref where I can use ref.current.querySelectorAll to do some magic. Now I need to add html to my dependencies in useEffect even though I am not explicitly using it. Is this a use case where I actually should disable the rule?

The idea is to intercept all the link clicks from the editorial content and track some analytics or do other relevant stuff.

This is a watered down example from a real component: https://codesandbox.io/s/8njp0pm8v2

[joelmoss]

I'm using react-redux, so when passing down an action creator in the props from mapDispatchToProps, and using that action creator in a hook, I get a exhaustive-deps warning.

So I can obviously add the redux action to the dependency array, but because the redux action is a function and never changes, this is unneccessary right?

[lukyth]

const onSubmit = React.useCallback(
  () => {
    props.onSubmit(emails);
  },
  [emails, props]
);

I expect the lint to fix the deps into [emails, props.onSubmit], but right now it always fix the deps into [emails, props].

1. A CodeSandbox demonstrating a minimal code example that still expresses your intent (not "foo bar" but actual UI pattern you're implementing).

https://codesandbox.io/s/xpr69pllmz

2. An explanation of the steps a user does and what you expect to see on the screen.

A user will add an email and invite those email to the app. I intentionally remove the rest of the UI to just the button because they're irrelevant to my issue.

3. An explanation of the intended API of your Hook/component.

My component has a single prop, onSubmit: (emails: string[]) => void. It'll be called with the emails state when a user submit the form.

---

EDIT: Answered in https://github.com/facebook/react/issues/14920#issuecomment-467494468

This is because technically props.foo() passes props itself as this to foo call. So foo might implicitly depend on props. We'll need a better message for this case though. The best practice is always destructuring.

[atomiks]

CodeSandbox

It doesn't consider that mount and update can be distinctly different when integrating when 3rd party libs. The update effect can't be included in the mount one (and removing the array altogether), because the instance shouldn't be destroyed on every render

[sylvainbaronnet]

Hi,

Not sure what's wrong with my code here :

const [client, setClient] = useState(0);

useEffect(() => {
    getClient().then(client => setClient(client));
  }, ['client']);

I got React Hook useEffect has a complex expression in the dependency array. Extract it to a separate variable so it can be statically checked

[gaearon]

@joelmoss @sylvainbaronnet I appreciate your feedback but I hid your comments because they didn't include the information we asked for at the top of the issue. That makes this discussion unnecessarily difficult for everyone because there's missing context. I'd be happy to continue the conversation if you post again and include all the relevant information (see the top post). Thank you for understanding.

[btholt]

1. CodeSandbox
2. User can select a first name and it forces the last name to change. User can select a last name and it does not force the first name to change.
3. There's a custom hook which returns a piece of state, a select component that updates that state, and the state hook's update method that updates the state programmatically. As demonstrated, I don't always want to use the updater function so I left it to the be last item in returned array.

I believe the code as-is shouldn't error. It does right now on line 35, saying that setLastName should be included in the array. Any thoughts on what to do about that? Am I doing something unexpected?

[joelmoss]

I totally understand, and I would normally do all that for you, but in my specific case, none of the code is unique to me. It's simply a question about whether the use of a function that is defined outside of the hook (ie. a redux action creator) and used within a hook should require that function to be added as a hook dep.

Happy to create a codesandbox if you still need more info. thx

[btholt]

@joelmoss I think my repro covers your case too.

[gaearon]

Yes, a CodeSandbox would still help. Please imagine what it's like to context switch between people's code snippets all day. It's a huge mental toll. None of them look the same. When you need to remember how people use action creators in Redux or some other concept external to React it's even harder. The problem may sound obvious to you but it's not at all obvious to me what you meant.

[sylvainbaronnet]

@gaearon I understand it makes sense, actually it worked for me because why I wanted to achieve was :

If you want to run an effect and clean it up only once (on mount and unmount), you can pass an empty array ([]) as a second argument.

Thanks a lot for the clarification. (and sorry for the off topic)

[robbestad]

Here's the code version of what I'm implementing. It don't look like much, but the pattern is 100% identical with my real code.

https://codesandbox.io/s/2x4q9rzwmp?fontsize=14

• An explanation of the steps a user does and what you expect to see on the screen.

Everything works great in this example! Except the linter issue.

• An explanation of the intended API of your Hook/component.

I've got several files like this, where I'm executing a function in the effect, but I only want it to run whenever some condition is met - for instance, changing the series Id. I don't want to include the function in the array.

This is what I get from the linter when running it on the sandbox code:

25:5   warning  React Hook useEffect has a missing dependency: 'fetchPodcastsFn'. Either include it or remove the dependency array  react-hooks/exhaustive-deps

My question is: is this how it should behave (either the linter rule, or the hook array rule)? Is there a more idiomatic way of describing this effect?

[tungv]

@svenanders, I'm curious about the reason why you don't want to include fetchPodcastsFn? Is it because you find it change on every render? If it is, you probably want to memoize that function or make it static (in case it doesn't have any parameters)

[robbestad]

For one - it's about clarity. When I look at the function, I want to easily understand when it should fire. If I see one id in the array, it's crystal clear. If I see that id and a bunch of functions, it becomes more muddled. I have to spend time and effort, possibly even debugging functions, to understand what's going on. My functions don't change on runtime, so I don't know if memoizing them would matter (this one in particular is a dispatch action that fires off an epic, eventually resulting in a state change).

[ferdaber]

https://codesandbox.io/s/4xym4yn9kx

• Steps

The user accesses a route on the page, but they're not a super user, so we want to redirect them away from the page. The props.navigate is injected via a router library so we don't actually want to use window.location.assign to prevent a page reload.

Everything works!

• Intended API

I put in the dependencies correctly as in the code sandbox, but the linter is telling me that the dependency list should have props instead of props.navigate. Why is that?

A tweet with screenshots! https://twitter.com/ferdaber/status/1098966716187582464

EDIT: one valid reason this could be buggy is if navigate() is a method that relies on a bound this, in which case technically if anything inside props changes then the stuff inside this will also change.

[ThisIsOstad]

CodeSandbox: https://codesandbox.io/s/711r1zmq50

Intended API:

This hook allows you to debounce any fast changing value. The debounced value will only reflect the latest value when the useDebounce hook has not been called for the specified time period. When used in conjunction with useEffect, as we do in the recipe, you can easily ensure that expensive operations like API calls are not executed too frequently.

Steps:

The example allows you to search the Marvel Comic API and uses useDebounce to prevent API calls from being fired on every keystroke.

IMO adding "everything" we use in dependencies array is not efficient.

For example consider the useDebounce Hook. In real world usages (at least in ours), the delay will not change after the first render, but we are checking if it's changed or not on every re-render. So, in this hook, the second argument of useEffect is better to be [value] instead of [value, delay].

Please don't think shallow equality checks are extremely cheap. They can help your app when placed strategically but just making every single component pure can actually make your app slower. Tradeoffs.

I think adding everything in dependencies array, has the same (or even worse) performance issue as using pure components everywhere. Since, there are many scenarios in which we know that some of the values we're using will not change, so we shouldn't add them in dependencies array, because as @gaearon said, shallow equality checks are not very cheap and this can make our app slower.

[knpwrs]

I have some feedback about enabling this rule to automatically work on custom hooks by convention.

I can see in the source code that there is some intent to allow people to specify a regex to capture custom hooks by name:

https://github.com/facebook/react/blob/ba708fa79b3db6481b7886f9fdb6bb776d0c2fb9/packages/eslint-plugin-react-hooks/src/ExhaustiveDeps.js#L490-L492

What would the React team think about an additional naming convention for custom hooks with dependency arrays? Hooks already follow the convention of being prefixed by use in order to be detected as a hook by this plugin. The convention I would propose for detecting custom hooks that rely on certain dependencies would be some sort of postfix, something like WithDeps, meaning a full custom hook name could be something like useCustomHookWithDeps. The WithDeps postfix would tell the plugin that the final array argument is one of dependencies.

The plugin could still additionally support regexes, but I think I'd see great benefit by allowing library authors to simply export their custom hooks postfixed with WithDeps rather than forcing library consumers to explicitly configure the plugin for any and all custom hooks 3rd-party or otherwise.

[nghiepdev]

It warning and auto remove of the custom equality check.

const myEqualityCheck = a => a.includes('@');

useCallback(
  () => {
    // ...
  },
  [myEqualityCheck(a)]
);

[MrLeebo]

For useEffect, I don't think the "unnecessary dependency" warning should appear because those "dependencies" will change how the effect fires.

Let's say I have two counters, parent and child:

• The counters can be incremented/decremented independently.
• I want to reset the child counter to zero when the parent counter changes.

Implementation:

  const [parent, setParent] = useState(0);
  const [child, setChild] = useState(0);

  useEffect(
    () => {
      setChild(0);
    },
    [parent] // "unnecessary dependency: 'parent'"
  );

The exhaustive-deps rule gives the warning React Hook useEffect has an unnecessary dependency: 'parent'. Either exclude it or remove the dependency array.

I don't think the linter should be making this suggestion because it changes how the app behaves.

1. CodeSandbox: https://codesandbox.io/s/ol6r9plkv5
2. Steps: When parent changes, child resets to zero.
3. Intention: Instead of flagging the dependency as unnecessary, the linter should recognize that parent changes the behavior of useEffect and shouldn't advise me to remove it.

[EDIT]

As a workaround, I can write something like

const [parent, setParent] = useState(0)
const [child, setChild] = useState(0)
const previousParent = useRef(parent)

useEffect(() => {
  if (parent !== previousParent.current) {
    setChild(0)
  }

  previousParent.current = parent
}, [parent])

But there is a "poetry" to the original snippet that I like.

I think the question comes down to whether we should interpret the dependency array as just a mechanism for performance optimizations vs actual component behavior. The React Hooks FAQ uses performance as an example for why you would use it, but I didn't interpret the example to mean you should only use the dependency array to improve performance, instead I saw it as a convenient way to skip the effect calls in general.

It's actually a pattern I've used a couple of times, to invalidate some internal cache:

useEffect(() => {
  if (props.invalidateCache) {
    cache.clear()
  }
}, [props.invalidateCache])

If I shouldn't be using it in this way, then feel free to just let me know and disregard this comment.

[eps1lon]

@MrLeebo What about

  const [parent, setParent] = useState(0);
  const [child, setChild] = useState(0);
  const updateChild = React.useCallback(() => setChild(0), [parent]);

  useEffect(
    () => {
      updateChild();
    },
    [updateChild] 
  );

I wouldn't understand that snippet of you either. The dependency can only be assumed based on your code but it might be a bug. While my proposal doesn't necessarily solve that it makes it at least more explicit.

It looks like this was previously solved via getDerivedStateFromProps? Does How do I implement getDerivedStateFromProps? help?

[gaearon]

@nghiepit I hid your comment because you ignored the required checklist in the first post (e.g. CodeSandbox). Please follow the checklist and post again. Thanks.

[MrLeebo]

@eps1lon You would have the exact same warning on useCallback, and I disagree with that pattern in general being an improvement over the original, but I don't want to derail the topic to talk about that. To clarify, I think the unnecessary dependency rule should be relaxed for useEffect or useLayoutEffect specifically, because they can contain effectful logic, but the rule should remain in place for useCallback, useMemo, etc.

[ksaldana1]

I'm running into some of the same issues/mental model questions that @MrLeebo has described here. My gut feeling is that the useEffect dependency rule can't be as strict. I have an incredibly contrived example that I was working on for a basic proof of concept idea. I know this code sucks and the idea isn't particularly useful, but I think it's a good illustration of the problem at hand. I think @MrLeebo expressed the question I am having quite well:

I think the question comes down to whether we should interpret the dependency array as just a mechanism for performance optimizations vs actual component behavior. The React Hooks FAQ uses performance as an example for why you would use it, but I didn't interpret the example to mean you should only use the dependency array to improve performance, instead I saw it as a convenient way to skip the effect calls in general.

https://codesandbox.io/s/5v9w81j244

I'd specifically look at the useEffect hook in useDelayedItems. Right now including the items property in the dependency array will cause a linting error, but removing that property or the array entirely gives you behavior you do not want.

The basic idea of the useDelayedItems hook is given an array of items and a config object (delay in ms, initial page size), I will initially be handed a subset of items depending on my configured page size. After config.delay has passed, the items will now be the full set of items. When it is handed a new set of items, the hook should re-run this "delay" logic. The idea is a very crude and dumb version of delayed rendering of large lists.

Thanks for all the work on these rules, it's been very helpful when developing. Even if my example is of questionable quality, I hope it gives some insight in how these operators may be ab/used.

EDIT: I have added a few clarifying comments inside the codesandbox around behavior intent. I hope the information is sufficient, but let me know if anything is still confusing.

[aweary]

What about a single value that combines other values?

Example: fullName is derived from firstName and lastName. We want to trigger the effect only when fullName changes (like when the user hits "Save") but also want to access the values it composes in the effect

Demo

Adding firstName or lastName to the dependencies would break things since we only want to run the effect after fullName has changed.

[ksaldana1]

@aweary I am not sure what value you are getting from the useEffect prop change indirection. It seems that your onClick should be handling that "effect".

https://codesandbox.io/s/0m4p3klpyw

As far as single values that combine other values, useMemo is probably going to be what you want. The delayed nature of the calculation in your example means it doesn't map exactly 1:1 with your linked behavior.

[bugzpodder]

I will create codesandbox links for these examples and edit this post.

I have an extremely simple rule: if the current tab changed, scroll to the top: https://codesandbox.io/s/m4nzvryrxj

useEffect(() => {
    window.scrollTo(0, 0);
  }, [activeTab]);

And I got React Hook useEffect has an unnecessary dependency: 'activeTab'. Either exclude it or remove the dependency array

Also, when I was migrating some components, I want to replicate the componentDidMount behavior:

useEffect(() => {
    ...
  }, []);

this rule is complaining hard about this. I am hesitant to add every dependency to the useEffect array.

Finally, if you declare a new inline function before useEffect, like this: https://codesandbox.io/s/nr7wz8qp7l

const foo = () => {...};
useEffect(() => {
    foo();
  }, []);

You get: React Hook useEffect has a missing dependency: 'foo'. Either include it or remove the dependency array I am not sure how I feel about adding foo to the dependencies list. In each render foo is a new function and useEffect would always get run?

[aweary]

@ksaldana1 putting the side effect inside the event handler isn't sufficient. That would cause the side effect to occur before the actual update is committed, which might cause the side effect to trigger more often than you want.

It also won't work when using useReducer because the updated state won't be available inside the event handler.

As far as single values that combine other values, useMemo is probably going to be what you want.

If this example used useMemo it would break because useMemo would derive a new fullName every time firstName or lastName changed. The behavior here is that fullName doesn't get updated until the Save button is clicked.

[henryqdineen]

@aweary Would something like this work? https://codesandbox.io/s/lxjm02j50m I'm very curious to see what the recommended implementation is.

I'm also curious to hear more about a couple things you stated. Can you point me to any more info about these?

Triggering and effect in handler:

that would cause the side effect to occur before the actual update is committed, which might cause the side effect to trigger more often than you want.

Using useReducer state in event handler:

the updated state won't be available.

Thanks!

[atomiks]

@bugzpodder kind of unrelated but the scroll call should be inside useLayoutEffect instead of useEffect. Currently there's a noticeable flicker when navigating to the new route

[siddharthkp]

Not sure if this is intentional or not:

When you call a function from props, the linter suggests adding the entire props object as a dependency.

Short version:

useEffect(function() {
  props.setLoading(true)
}, [props.setLoading])

// ⚠️ React Hook useEffect has a missing dependency: 'props'.

Full version: Codesandbox

[joelmoss]

Trying again... but simpler this time ;)

When passing down a function in the props, or indeed from anywhere, and using that function inside a hook, I get a exhaustive-deps warning.

So I can obviously add the function to the dependency array, but because it is a function and never changes, this is unneccessary right?

-> Sandbox

Hope that is everything you need, but I simply forked @siddharthkp's sandbox as that demonstrated what I meant.

thx.

[gaearon]

So I can obviously add the function to the dependency array, but because it is a function and never changes, this is unneccessary right?

This is not correct; functions can change all the time when the parent re-renders.

[gaearon]

@siddharthkp

When you call a function from props, the linter suggests adding the entire props object as a dependency.

This is because technically props.foo() passes props itself as this to foo call. So foo might implicitly depend on props. We'll need a better message for this case though. The best practice is always destructuring.

[joelmoss]

functions can change all the time when the parent re-renders.

Sure, but if the function were defined elsewhere and not from a parent component, it wouldn't ever change.

[gaearon]

Sure, but if the function were defined elsewhere and not from a parent component, it wouldn't ever change.

If you import it directly, the lint rule won't ask you to add it to the effect deps. Only if it's in the render scope. If it's in the render scope then the lint rule doesn't know where it's coming from. Even if it's not dynamic today, it could be tomorrow when somebody changes the parent component. So specifying it is the right default. It doesn't hurt to specify it if it's static anyway.

[joelmoss]

thx @gaearon

[lukyth]

This is because technically props.foo() passes props itself as this to foo call. So foo might implicitly depend on props. We'll need a better message for this case though. The best practice is always destructuring.

That answers my question as well. Thank you! 😄

[thsorens]

https://codesandbox.io/s/98z62jkyro

So i am creating a library for handling input validation by registering all inputs using an api exposed in a context for each input to register themselves. I created a custom hook called useRegister.

Ex:

 useEffect(
    () => {
      register(props.name, props.rules || []);
      console.log("register");
      return function cleanup() {
        console.log("cleanup");
        unregister(props.name);
      };
    },
    [props.name, props.rules, register, unregister]
  );

When forcing props.rules to be a part of the dependencies, it seems to end up in an infinite render-loop. Props.rules is an array of functions to be registered as validators. I have only detected this issue when providing arrays as dependencies. In my codesandbox, you can see that it loops opening the console.

Just having props.name as dependency makes it work as intended. Enforcing the dependencies will as other pointed out change the behavior of the application, and in this occasion the side-effects are severe.

[gaearon]

@bugzpodder

Re: https://github.com/facebook/react/issues/14920#issuecomment-467212561

useEffect(() => {
    window.scrollTo(0, 0);
  }, [activeTab]);

This seems like a legit case. I'm going to relax the warning to allow extraneous deps for effects only. (But not for useMemo or useCallback.)

Also, when I was migrating some components, I want to replicate the componentDidMount behavior: this rule is complaining hard about this. I am hesitant to add every dependency to the useEffect array.

Sorry, you didn't add any example for that so we lost the opportunity to discuss that. That's exactly why the OP post asks to specify a concrete UI example. You did for the first point but not this one. I'm happy to discuss it when you add a concrete example for it. The specifics really depend on it.

Finally, if you declare a new inline function before useEffect, like this: https://codesandbox.io/s/nr7wz8qp7l

In this case the easy fix is to move doSomething into the effect. Then you don't need to declare it. Alternatively, you can useCallback around doSomething. I'm open to relaxing the rule to allow omitting functions that only use declared deps. But this can get confusing if you have a function calling another function, and add a prop or state into one of those functions. Suddenly all effect deps using it transitively have to be updated. It can get confusing.

[jaydenseric]

I don't know if this is a feature request for a new rule, or something that could be improved about exhaustive-deps…

import React from 'react'
import emitter from './thing'

export const Foo = () => {
  const onChange = () => {
    console.log('Thing changed')
  }

  React.useEffect(() => {
    emitter.on('change', onChange)
    return () => emitter.off('change', onChange)
  }, [onChange])

  return <div>Whatever</div>
}

Because the onChange function is created every render, the useEffect hook [onChange] argument is redundant, and might as well be removed:

   React.useEffect(() => {
     emitter.on('change', onChange)
     return () => emitter.off('change', onChange)
-  }, [onChange])
+  })

The linter could detect that, and advise you to delete the array argument.

There have been situations where I have been maintaining a list of array items, only to realise one or more of them were being created and invalidating the hook every render anyway.

[gaearon]

Just released eslint-plugin-react-hooks@1.4.0 with a few fixes and better messages for this rule. Nothing groundbreaking but a few cases should be solved. I'll be looking at the rest next week.

[gaearon]

I also posted first possible step for omitting "safe" function deps here: https://github.com/facebook/react/pull/14996. (See tests.) If you have ideas about useful heuristics and how to guide people to right fixes please comment on the PR.

[markozxuu]

@gaearon Excellent idea. This will definitely be useful to have a better style when using hooks 🙏

[WebDeg-Brian]

@gaearon It still doesn't work in case the action comes from prop. Consider this example:

In which setScrollTop is a redux action.

[jamesmosier]

In this example in the Slider component, I am using useEffect to wait until the DOM is available so I can mount the noUiSlider component. I therefore pass in [sliderElement] to ensure that the ref is available in the DOM when the effect runs. We server render our components as well, so this also ensures the DOM is available before rendering. The other props that I use in useEffect (i.e. min, max, onUpdate, etc) are constants and therefore I don't see a need for them to be passed in to the effect.

---

Here's the effect as seen in codesandbox:

const { min, max, step } = props;
const sliderElement = useRef();

useEffect(() => {
  if (!sliderElement.current) {
    return;
  }

  const slider = sliderElement.current;
  noUiSlider.create(slider, {
    connect: true,
    start: [min, max],
    step,
    range: {
      min: [min],
      max: [max],
    },
  });

  if (props.onUpdate) {
    slider.noUiSlider.on('update', props.onUpdate);
  }

  if (props.onChange) {
    slider.noUiSlider.on('change', props.onChange);
  }
}, [sliderElement]);

[gaearon]

@WebDeg-Brian I can’t help you without a full CodeSandbox demo. Sorry. See the top post.

[gaearon]

I posted a bit about the common “functions never change” misconception:

https://overreacted.io/how-are-function-components-different-from-classes/

Not quite the same topic, but relevant to this rule.

[sag1v]

Hi @gaearon, Here is the example you asked me to post here (from tweeter) :)

Basically i'm trying to convert my library react-trap to hooks. This is just a trap for events, outside / inside an element.

My problem is that if useEffect is not depended on the state value (trapped), it sometimes being stale. I wrote some comments and logs to demonstrate. Look at the useTrap.js file, the comments and logs are in useEffect and preventDefaultHelper functions.

To my knowledge, if a value is not inside useEffect then it shouldn't be part of its dependencies (correct me if i'm wrong).

1. A CodeSandbox
2. Steps: A user click inside the box to make it active, and outside to make it not active, the user can also click with the right mouse button, though for the first click it shouldn't trigger the context menu (e.preventDefault). When i say "first click" i mean the first click that changes the state. Given an active box, a right click outside of it will change the state to "not active" and prevent the context menu. another click outside wont affect the state, hence the context menu should appear.

I hope i'm being clear here and the use case is understandable, please let me know if i need to provide more info. Thanks!