Skip to content

Warn on mount when deps are not an array #15018

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom
Merged
merged 3 commits into from
Mar 5, 2019
Merged

Warn on mount when deps are not an array #15018

merged 3 commits into from
Mar 5, 2019

Conversation

gaearon
Copy link
Collaborator

@gaearon gaearon commented Mar 5, 2019

It's confusing that passing a string argument doesn't warn (since it has length) but eventually can crash (#15016). This adds early validation. I only check on mount to avoid the overhead on updates. If you get it wrong, you'll likely get it wrong on both mounts and updates.

@gaearon gaearon mentioned this pull request Mar 5, 2019
Closed
@sizebot
Copy link

sizebot commented Mar 5, 2019 •
edited
Loading

Details of bundled changes.

Comparing: ff596e3...72b1308

react-dom

File Filesize Diff Gzip Diff Prev Size Current Size Prev Gzip Current Gzip ENV
react-dom.development.js +0.1% +0.1% 775.11 KB 775.74 KB 176.39 KB 176.53 KB UMD_DEV
react-dom.profiling.min.js 0.0% 0.0% 107.9 KB 107.9 KB 34.73 KB 34.74 KB UMD_PROFILING
react-dom.development.js +0.1% +0.1% 769.49 KB 770.12 KB 174.87 KB 175.02 KB NODE_DEV
react-dom.production.min.js 0.0% -0.0% 104.87 KB 104.87 KB 33.55 KB 33.54 KB NODE_PROD
ReactDOM-dev.js +0.1% +0.1% 792.36 KB 793.03 KB 176.01 KB 176.15 KB FB_WWW_DEV
ReactDOM-profiling.js 0.0% 0.0% 326.91 KB 326.91 KB 60.08 KB 60.08 KB FB_WWW_PROFILING
react-dom-unstable-fire.development.js +0.1% +0.1% 775.46 KB 776.08 KB 176.53 KB 176.67 KB UMD_DEV
react-dom-unstable-fire.profiling.min.js 0.0% 0.0% 107.92 KB 107.92 KB 34.74 KB 34.74 KB UMD_PROFILING
react-dom-unstable-fire.development.js +0.1% +0.1% 769.84 KB 770.46 KB 175.01 KB 175.15 KB NODE_DEV
react-dom-unstable-fire.production.min.js 0.0% -0.0% 104.89 KB 104.89 KB 33.55 KB 33.55 KB NODE_PROD
react-dom-unstable-fire.profiling.min.js 0.0% 0.0% 107.98 KB 107.98 KB 34.17 KB 34.17 KB NODE_PROFILING
ReactFire-dev.js +0.1% +0.1% 791.57 KB 792.24 KB 175.97 KB 176.11 KB FB_WWW_DEV
react-dom-test-utils.development.js 0.0% 0.0% 47.06 KB 47.06 KB 12.99 KB 12.99 KB UMD_DEV
react-dom-test-utils.production.min.js 0.0% -0.0% 10.27 KB 10.27 KB 3.8 KB 3.8 KB UMD_PROD
react-dom-test-utils.production.min.js 0.0% 0.0% 10.05 KB 10.05 KB 3.73 KB 3.73 KB NODE_PROD
react-dom-unstable-native-dependencies.development.js 0.0% 0.0% 60.61 KB 60.61 KB 15.92 KB 15.92 KB UMD_DEV
react-dom-unstable-native-dependencies.production.min.js 0.0% -0.0% 11.01 KB 11.01 KB 3.81 KB 3.81 KB UMD_PROD
react-dom-unstable-native-dependencies.development.js 0.0% 0.0% 60.28 KB 60.28 KB 15.79 KB 15.79 KB NODE_DEV
react-dom-unstable-native-dependencies.production.min.js 0.0% -0.0% 10.75 KB 10.75 KB 3.71 KB 3.71 KB NODE_PROD
react-dom-server.browser.development.js 0.0% -0.0% 127.39 KB 127.39 KB 33.95 KB 33.95 KB UMD_DEV
react-dom-server.node.development.js 0.0% -0.0% 125.58 KB 125.58 KB 33.56 KB 33.56 KB NODE_DEV
react-dom-unstable-fizz.browser.production.min.js 0.0% 🔺+0.1% 1.21 KB 1.21 KB 705 B 706 B UMD_PROD
react-dom-unstable-fizz.browser.development.js 0.0% +0.1% 3.45 KB 3.45 KB 1.39 KB 1.39 KB NODE_DEV
react-dom-unstable-fizz.browser.production.min.js 0.0% 🔺+0.2% 1.05 KB 1.05 KB 636 B 637 B NODE_PROD
react-dom-unstable-fizz.node.development.js 0.0% +0.1% 3.7 KB 3.7 KB 1.42 KB 1.42 KB NODE_DEV
react-dom-unstable-fizz.node.production.min.js 0.0% 🔺+0.2% 1.1 KB 1.1 KB 666 B 667 B NODE_PROD

react-art

File Filesize Diff Gzip Diff Prev Size Current Size Prev Gzip Current Gzip ENV
react-art.development.js +0.1% +0.1% 550.78 KB 551.41 KB 119.52 KB 119.67 KB UMD_DEV
react-art.development.js +0.1% +0.1% 481.66 KB 482.29 KB 102.26 KB 102.4 KB NODE_DEV
react-art.production.min.js 0.0% -0.0% 61.83 KB 61.83 KB 19 KB 19 KB NODE_PROD
ReactART-dev.js +0.1% +0.1% 490.82 KB 491.5 KB 101.49 KB 101.63 KB FB_WWW_DEV
ReactART-prod.js 0.0% -0.0% 194.94 KB 194.94 KB 33.13 KB 33.13 KB FB_WWW_PROD

react-native-renderer

File Filesize Diff Gzip Diff Prev Size Current Size Prev Gzip Current Gzip ENV
ReactNativeRenderer-dev.js +0.1% +0.1% 617.92 KB 618.59 KB 132.18 KB 132.32 KB RN_FB_DEV
ReactNativeRenderer-prod.js 0.0% 0.0% 246.25 KB 246.25 KB 43.12 KB 43.12 KB RN_FB_PROD
ReactNativeRenderer-profiling.js 0.0% -0.0% 252.43 KB 252.43 KB 44.49 KB 44.49 KB RN_FB_PROFILING
ReactNativeRenderer-dev.js +0.1% +0.1% 617.83 KB 618.5 KB 132.14 KB 132.29 KB RN_OSS_DEV
ReactNativeRenderer-prod.js 0.0% 0.0% 246.26 KB 246.26 KB 43.12 KB 43.12 KB RN_OSS_PROD
ReactFabric-dev.js +0.1% +0.1% 608.77 KB 609.44 KB 129.89 KB 130.04 KB RN_FB_DEV
ReactFabric-dev.js +0.1% +0.1% 608.68 KB 609.35 KB 129.85 KB 129.99 KB RN_OSS_DEV
ReactFabric-prod.js 0.0% 0.0% 239.33 KB 239.33 KB 41.67 KB 41.67 KB RN_OSS_PROD
ReactFabric-profiling.js 0.0% 0.0% 244.67 KB 244.67 KB 42.99 KB 43 KB RN_OSS_PROFILING

react-test-renderer

File Filesize Diff Gzip Diff Prev Size Current Size Prev Gzip Current Gzip ENV
react-test-renderer.development.js +0.1% +0.1% 490.68 KB 491.3 KB 103.96 KB 104.1 KB UMD_DEV
react-test-renderer.production.min.js 0.0% -0.0% 62.92 KB 62.92 KB 19.32 KB 19.32 KB UMD_PROD
react-test-renderer.development.js +0.1% +0.1% 486.22 KB 486.84 KB 102.81 KB 102.95 KB NODE_DEV
react-test-renderer.production.min.js 0.0% -0.0% 62.59 KB 62.59 KB 19.06 KB 19.06 KB NODE_PROD
ReactTestRenderer-dev.js +0.1% +0.1% 496.2 KB 496.87 KB 102.44 KB 102.58 KB FB_WWW_DEV
react-test-renderer-shallow.development.js 0.0% 0.0% 37.2 KB 37.2 KB 9.5 KB 9.5 KB UMD_DEV
react-test-renderer-shallow.development.js 0.0% 0.0% 31.5 KB 31.5 KB 8.14 KB 8.14 KB NODE_DEV

react-reconciler

File Filesize Diff Gzip Diff Prev Size Current Size Prev Gzip Current Gzip ENV
react-reconciler.development.js +0.1% +0.1% 479.71 KB 480.33 KB 100.7 KB 100.84 KB NODE_DEV
react-reconciler-persistent.development.js +0.1% +0.1% 477.91 KB 478.53 KB 99.99 KB 100.13 KB NODE_DEV
react-reconciler-reflection.development.js 0.0% 0.0% 15.76 KB 15.76 KB 4.98 KB 4.98 KB NODE_DEV

Generated by 🚫 dangerJS

bvaughn
bvaughn approved these changes Mar 5, 2019
View reviewed changes
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.

👍

gaearon added 2 commits March 5, 2019 17:15
@gaearon
Check other Hooks
48da01e
@gaearon
I can't figure out how to fix error/warning nesting lint
72b1308 
But it doesn't really matter much because we test other cases in the other test.
@gaearon gaearon removed the CLA Signed label Mar 5, 2019
@gaearon gaearon merged commit fd557d4 into facebook:master Mar 5, 2019
@gaearon gaearon deleted the clearer-warning branch March 5, 2019 17:41
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@bvaughn bvaughn

@acdlite acdlite

@sophiebits sophiebits

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@gaearon @sizebot @bvaughn @facebook-github-bot