Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Allow forwardRef to be composed with pure #13897

Closed
wants to merge 1 commit into from

Conversation

sebmarkbage
Copy link
Collaborator

This uses a different approach than the moddable approach because that approach also started getting quite complex and affects tags and symbols.

Instead, I just add a compare field to forwardRef's element type and use undefined to represent the lack of pure wrapper and null to represent the default.

This way we don't affect the perf of pure and when we drop forwardRef we can just drop this path.

This also handles the composition of multiple pure wrappers with custom comparisons.

@sizebot
Copy link

sizebot commented Oct 19, 2018

React: size: 🔺+1.9%, gzip: 🔺+1.2%

ReactDOM: size: 🔺+0.2%, gzip: 🔺+0.1%

Details of bundled changes.

Comparing: fa65c58...c19335b

react

File Filesize Diff Gzip Diff Prev Size Current Size Prev Gzip Current Gzip ENV
react.development.js +0.7% +0.6% 94.03 KB 94.67 KB 24.73 KB 24.88 KB UMD_DEV
react.production.min.js 🔺+1.9% 🔺+1.2% 11.69 KB 11.92 KB 4.62 KB 4.68 KB UMD_PROD
react.development.js +1.1% +0.9% 56.74 KB 57.38 KB 15.24 KB 15.39 KB NODE_DEV
react.production.min.js 🔺+3.6% 🔺+2.3% 6.11 KB 6.33 KB 2.6 KB 2.66 KB NODE_PROD
React-dev.js +1.2% +1.3% 53.11 KB 53.76 KB 14.4 KB 14.58 KB FB_WWW_DEV
React-prod.js 🔺+4.5% 🔺+3.1% 13.95 KB 14.57 KB 3.82 KB 3.94 KB FB_WWW_PROD
React-profiling.js +4.5% +3.1% 13.95 KB 14.57 KB 3.82 KB 3.94 KB FB_WWW_PROFILING
react.profiling.min.js +1.6% +1.0% 13.85 KB 14.07 KB 5.14 KB 5.19 KB UMD_PROFILING

react-dom

File Filesize Diff Gzip Diff Prev Size Current Size Prev Gzip Current Gzip ENV
react-dom.development.js +0.1% 0.0% 677.48 KB 678.06 KB 157.36 KB 157.41 KB UMD_DEV
react-dom.production.min.js 🔺+0.2% 🔺+0.1% 97.69 KB 97.84 KB 31.88 KB 31.92 KB UMD_PROD
react-dom.development.js +0.1% 0.0% 672.82 KB 673.4 KB 155.96 KB 156 KB NODE_DEV
react-dom.production.min.js 🔺+0.2% 🔺+0.1% 97.71 KB 97.86 KB 31.44 KB 31.47 KB NODE_PROD
ReactDOM-dev.js +0.1% 0.0% 690.83 KB 691.5 KB 156.65 KB 156.71 KB FB_WWW_DEV
ReactDOM-prod.js 🔺+0.3% 🔺+0.1% 295.66 KB 296.41 KB 53.91 KB 53.98 KB FB_WWW_PROD
react-dom.profiling.min.js +0.1% +0.1% 100.11 KB 100.26 KB 31.79 KB 31.83 KB NODE_PROFILING
ReactDOM-profiling.js +0.3% +0.1% 300.05 KB 300.8 KB 54.95 KB 55.01 KB FB_WWW_PROFILING
react-dom.profiling.min.js +0.1% +0.1% 100.04 KB 100.19 KB 32.29 KB 32.32 KB UMD_PROFILING

react-art

File Filesize Diff Gzip Diff Prev Size Current Size Prev Gzip Current Gzip ENV
react-art.development.js +0.1% 0.0% 464.94 KB 465.51 KB 103.19 KB 103.23 KB UMD_DEV
react-art.production.min.js 🔺+0.2% 🔺+0.2% 89.64 KB 89.79 KB 27.51 KB 27.57 KB UMD_PROD
react-art.development.js +0.1% +0.1% 396.71 KB 397.29 KB 86.07 KB 86.12 KB NODE_DEV
react-art.production.min.js 🔺+0.3% 🔺+0.1% 54.62 KB 54.76 KB 16.79 KB 16.81 KB NODE_PROD
ReactART-dev.js +0.2% +0.1% 401.09 KB 401.77 KB 84.53 KB 84.58 KB FB_WWW_DEV
ReactART-prod.js 🔺+0.5% 🔺+0.2% 168.79 KB 169.61 KB 28.25 KB 28.32 KB FB_WWW_PROD

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% 410.18 KB 410.75 KB 88.9 KB 88.95 KB UMD_DEV
react-test-renderer.production.min.js 🔺+0.3% 🔺+0.2% 56.08 KB 56.23 KB 17.17 KB 17.19 KB UMD_PROD
react-test-renderer.development.js +0.1% +0.1% 405.75 KB 406.33 KB 87.8 KB 87.85 KB NODE_DEV
react-test-renderer.production.min.js 🔺+0.3% 🔺+0.1% 55.79 KB 55.94 KB 16.97 KB 16.98 KB NODE_PROD
ReactTestRenderer-dev.js +0.2% +0.1% 410.47 KB 411.15 KB 86.61 KB 86.67 KB FB_WWW_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% 394.41 KB 394.99 KB 84.51 KB 84.56 KB NODE_DEV
react-reconciler.production.min.js 🔺+0.3% 🔺+0.1% 55.68 KB 55.83 KB 16.6 KB 16.62 KB NODE_PROD
react-reconciler-persistent.development.js +0.1% +0.1% 393.02 KB 393.59 KB 83.96 KB 84 KB NODE_DEV
react-reconciler-persistent.production.min.js 🔺+0.3% 🔺+0.1% 55.69 KB 55.84 KB 16.6 KB 16.62 KB NODE_PROD

react-native-renderer

File Filesize Diff Gzip Diff Prev Size Current Size Prev Gzip Current Gzip ENV
ReactNativeRenderer-dev.js +0.1% 0.0% 527.69 KB 528.36 KB 115.48 KB 115.54 KB RN_FB_DEV
ReactNativeRenderer-prod.js 🔺+0.4% 🔺+0.2% 223.6 KB 224.44 KB 38.56 KB 38.65 KB RN_FB_PROD
ReactNativeRenderer-dev.js +0.1% 0.0% 527.4 KB 528.07 KB 115.4 KB 115.46 KB RN_OSS_DEV
ReactNativeRenderer-prod.js 🔺+0.4% 🔺+0.2% 223.62 KB 224.45 KB 38.56 KB 38.65 KB RN_OSS_PROD
ReactFabric-dev.js +0.1% 0.0% 517.91 KB 518.59 KB 113.06 KB 113.11 KB RN_FB_DEV
ReactFabric-prod.js 🔺+0.4% 🔺+0.2% 218.18 KB 219.02 KB 37.29 KB 37.37 KB RN_FB_PROD
ReactFabric-dev.js +0.1% +0.1% 517.95 KB 518.62 KB 113.07 KB 113.13 KB RN_OSS_DEV
ReactFabric-prod.js 🔺+0.4% 🔺+0.2% 218.22 KB 219.06 KB 37.3 KB 37.38 KB RN_OSS_PROD
ReactNativeRenderer-profiling.js +0.4% +0.2% 229.2 KB 230.04 KB 39.93 KB 40.01 KB RN_OSS_PROFILING
ReactFabric-profiling.js +0.4% +0.2% 222.94 KB 223.78 KB 38.59 KB 38.67 KB RN_OSS_PROFILING
ReactNativeRenderer-profiling.js +0.4% +0.2% 229.18 KB 230.02 KB 39.93 KB 40.01 KB RN_FB_PROFILING
ReactFabric-profiling.js +0.4% +0.2% 222.9 KB 223.74 KB 38.57 KB 38.66 KB RN_FB_PROFILING

scheduler

File Filesize Diff Gzip Diff Prev Size Current Size Prev Gzip Current Gzip ENV
scheduler.development.js n/a n/a 0 B 19.17 KB 0 B 5.74 KB UMD_DEV
scheduler.production.min.js n/a n/a 0 B 3.16 KB 0 B 1.53 KB UMD_PROD

Generated by 🚫 dangerJS

This uses a different approach than the moddable approach because that
approach also started getting quite complex and affects tags and symbols.

This way we don't affect the perf of pure and when we drop forwardRef
we can just drop this path.

This also handles the composition of multiple pure wrappers with custom
comparisons.
@aweary aweary mentioned this pull request Oct 19, 2018
@sebmarkbage
Copy link
Collaborator Author

Superseded by #13903 which is better since it allows any type to be passed to pure.

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.

4 participants