Skip to content

Conversation

@gaearon
Copy link
Collaborator

@gaearon gaearon commented Feb 27, 2019

This makes cases like

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

not warn.

However, it still warns for unused useCallback/useMemo deps.

Sebastian didn't disagree with this.

Resolves #14920 (comment).

This makes cases like

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

not warn.

However, it still warns for unused useCallback/useMemo deps.
@sizebot
Copy link

sizebot commented Feb 27, 2019

Details of bundled changes.

Comparing: 00748c5...a65912d

eslint-plugin-react-hooks

File Filesize Diff Gzip Diff Prev Size Current Size Prev Gzip Current Gzip ENV
eslint-plugin-react-hooks.development.js +0.9% +1.0% 49.28 KB 49.71 KB 11.62 KB 11.73 KB NODE_DEV
eslint-plugin-react-hooks.production.min.js 🔺+0.4% 🔺+0.1% 12.31 KB 12.37 KB 4.47 KB 4.47 KB NODE_PROD
ESLintPluginReactHooks-dev.js +0.8% +0.9% 52.81 KB 53.24 KB 11.95 KB 12.06 KB FB_WWW_DEV

Generated by 🚫 dangerJS

Copy link
Contributor

@bvaughn bvaughn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sebastian didn't disagree with this.

😆

I have somewhat mixed feelings about this change, but I trust your intuition that this is the right tradeoff based on the real world use cases you've been looking at 👍

@gaearon gaearon merged commit 3ada82b into facebook:master Feb 27, 2019
@gaearon gaearon deleted the fx-tweaks branch February 27, 2019 16:59
@Janpot
Copy link

Janpot commented Feb 27, 2019

@gaearon FYI: Not sure if this was the intention, but your PR comment automatically closed #14920

@bvaughn
Copy link
Contributor

bvaughn commented Feb 27, 2019

Reopened~

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants