oliviertassinari / babel-plugin-transform-react-remove-prop-types

Remove unnecessary React propTypes from the production build. :balloon:
MIT License
897 stars 61 forks source link

Is it supports react-native? #182

Closed kesha-antonov closed 4 years ago

kesha-antonov commented 5 years ago

Hey! Can't get it working. I've tried .babelrc, babel.config.js

Maybe I need some special setup opposite to reactjs?

kesha-antonov commented 5 years ago

Found the cause.

This works:

var CONSTANTS, PropTypes, React, StatusBar, View, WebView, colors, inject, observer, propTypes, stylesCommon, stylesConstants, withMappedNavigationProps;

React = require('react');

({View, StatusBar} = require('react-native'));

({WebView} = require('react-native-webview'));

propTypes = require('prop-types');

stylesConstants = require('./styles/constants').default;

stylesCommon = require('./styles/common').default;

colors = require('./styles/colors').default;

CONSTANTS = require('./CONSTANTS').default;

({inject, observer, PropTypes} = require('mobx-react/native'));

({withMappedNavigationProps} = require('react-navigation-props-mapper'));

var CustomWebView = class CustomWebView extends React.Component {
  render() {
    ...
  }

};

CustomWebView.propTypes = {
  store: PropTypes.observableObject.isRequired,
  uri: propTypes.string.isRequired
};

CustomWebView = inject('store')(observer = CustomWebView);

CustomWebView = withMappedNavigationProps()(CustomWebView);

export default CustomWebView;

This doesn't work:

var CONSTANTS, CustomWebView, PropTypes, React, StatusBar, View, WebView, colors, inject, observer, propTypes, stylesCommon, stylesConstants, withMappedNavigationProps;

React = require('react');

({View, StatusBar} = require('react-native'));

({WebView} = require('react-native-webview'));

propTypes = require('prop-types');

stylesConstants = require('./styles/constants').default;

stylesCommon = require('./styles/common').default;

colors = require('./styles/colors').default;

CONSTANTS = require('./CONSTANTS').default;

({inject, observer, PropTypes} = require('mobx-react/native'));

({withMappedNavigationProps} = require('react-navigation-props-mapper'));

CustomWebView = class CustomWebView extends React.Component {
  render() {
    ...
  }

};

CustomWebView.propTypes = {
  store: PropTypes.observableObject.isRequired,
  uri: propTypes.string.isRequired
};

CustomWebView = inject('store')(observer = CustomWebView);

CustomWebView = withMappedNavigationProps()(CustomWebView);

export default CustomWebView;

Difference in var CustomWebView = ... vs CustomWebView = ...

oliviertassinari commented 5 years ago

@kesha-antonov Is this transpiled code? Is the plugin execution order correct?

kesha-antonov commented 5 years ago

@oliviertassinari Yeah, transpiled from coffeescript. And then passed with other JS files to react-native packager.

I've tried to minimize code for reproduce:

1) Doesn't work:

var CustomWebView;

React = require('react');

CustomWebView = class CustomWebView extends React.Component {
  render() {
    return <View />;
  }

};

CustomWebView.propTypes = {
  foo: ''
};

export default CustomWebView;

2) Works:

// var CustomWebView;

React = require('react');

var CustomWebView = class CustomWebView extends React.Component {
  render() {
    return <View />;
  }

};

CustomWebView.propTypes = {
  foo: ''
};

export default CustomWebView;

3) Doesn't work if in render I return null instead of <View />

// var CustomWebView;

React = require('react');

var CustomWebView = class CustomWebView extends React.Component {
  render() {
    return null;
  }

};

CustomWebView.propTypes = {
  foo: ''
};

export default CustomWebView;
kesha-antonov commented 5 years ago

Plugin works for other files not from coffeescript. I think we can add these to tests and update parsing

oliviertassinari commented 5 years ago

We shouldn't support CoffeeScript: https://www.rank2traffic.com/coffeescript.org. However, we should probably support any valid JavaScript output. If you want to work on the test case and fix, I would be happy to review it.

kesha-antonov commented 5 years ago

@oliviertassinari I'll write few tests

kesha-antonov commented 5 years ago

@oliviertassinari I added fixtures https://github.com/kesha-antonov/babel-plugin-transform-react-remove-prop-types/commit/bdcb988947ea006fe41c551c9e75b05ede300f15 Can you like at it if you have time?

Meanwhile I'm trying to understand the code

oliviertassinari commented 5 years ago

Thanks. I won't have time for it, maybe someone else will :).

kesha-antonov commented 5 years ago

Too hard for me. Don't understand how to fix it For

var React, propTypes, Foo;

React = require('react');
propTypes = require('prop-types');

Foo = class Foo extends React.Component {
  render() {
    return <View />;
  }

};

Foo.propTypes = {
  bar: PropTypes.string.isRequired,
}

export default Foo;

I see

binding.path.node.init Node {
  type: 'VariableDeclarator',
  start: 22,
  end: 26,
  loc:
   SourceLocation {
     start: Position { line: 1, column: 22 },
     end: Position { line: 1, column: 26 } },
  id:
   Node {
     type: 'Identifier',
     start: 22,
     end: 26,
     loc:
      SourceLocation { start: [Position], end: [Position], identifierName: 'Foo' },
     name: 'Foo' },
  init: null }