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

[ClickAwayListener] Hide react-event-listener #15420

Merged

Conversation

oliviertassinari
Copy link
Member

@oliviertassinari oliviertassinari commented Apr 19, 2019

Closes #15126

The new react hooks API makes the usage of react-event-listener less interesting. I think that it would be great to remove this dependency. It might yield a small bundle size gain (0.7 KB gzipped): https://bundlephobia.com/result?p=react-event-listener@0.6.6.

@joshwooding
Copy link
Member

The new react hooks API makes the usage of react-event-listener less interesting. I think that it would be great to remove this dependency.

During the refactor to hooks I'm removing EventListener where I can in favour of hooks

@mui-pr-bot
Copy link

mui-pr-bot commented Apr 19, 2019

Details of bundle changes.

Comparing: f132c3c...e510c1e

bundle parsed diff gzip diff prev parsed current parsed prev gzip current gzip
@material-ui/core -0.02% -0.02% 311,983 311,919 84,286 84,266
@material-ui/core/Paper 0.00% +0.04% 🔺 67,279 67,279 19,963 19,970
@material-ui/core/Paper.esm 0.00% -0.01% 60,640 60,640 18,883 18,881
@material-ui/core/Popper 0.00% 0.00% 34,907 34,907 11,809 11,809
@material-ui/core/Textarea 0.00% 0.00% 5,327 5,327 2,291 2,291
@material-ui/core/styles/createMuiTheme 0.00% 0.00% 15,898 15,898 5,771 5,771
@material-ui/core/useMediaQuery 0.00% 0.00% 2,106 2,106 973 973
@material-ui/lab 0.00% 0.00% 141,612 141,612 42,721 42,722
@material-ui/styles 0.00% +0.03% 🔺 50,833 50,833 15,012 15,017
@material-ui/system 0.00% -0.03% 11,765 11,765 3,929 3,928
Button 0.00% 0.00% 85,621 85,621 25,584 25,585
Modal 0.00% 0.00% 79,901 79,901 24,015 24,016
colorManipulator 0.00% 0.00% 3,904 3,904 1,543 1,543
docs.landing 0.00% 0.00% 50,908 50,908 11,210 11,210
docs.main -0.01% -0.04% 647,735 647,656 201,782 201,698
packages/material-ui/build/umd/material-ui.production.min.js -0.02% -0.02% 293,282 293,220 82,330 82,314

Generated by 🚫 dangerJS against e510c1e

@@ -114,6 +114,11 @@ ClickAwayListener.propTypes = {
touchEvent: PropTypes.oneOf(['onTouchStart', 'onTouchEnd', false]),
};

if (process.env.NODE_ENV !== 'production') {
// eslint-disable-next-line
ClickAwayListener['propTypes' + ''] = exactProp(ClickAwayListener.propTypes);
Copy link
Member

Choose a reason for hiding this comment

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

Guess this tricks babel-plugin-remove-prop-types?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, you are correct. While I didn't try without it in this specific case.

@oliviertassinari oliviertassinari merged commit 6f8082f into mui:next Apr 21, 2019
@oliviertassinari oliviertassinari deleted the hide-react-event-listener branch April 21, 2019 08:36
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.

[ClickAwayListener] Dependency on EventListener?
4 participants