RealOrangeOne / react-native-mock

A fully mocked and test-friendly version of react native (maintainers wanted)
MIT License
571 stars 153 forks source link

PropType warnings with RN 0.32.0 #91

Closed jasonfma closed 8 years ago

jasonfma commented 8 years ago

I've recently upgraded to RN 0.32.0 and now I'm getting warnings in all my tests.

e.g.

...

Warning: You are manually calling a React.PropTypes validation function for the `style` prop on `View`. This is deprecated and will not work in the next major version. You may be seeing this warning due to a third-party PropTypes library. See https://fb.me/react-warning-dont-call-proptypes for details.

Warning: You are manually calling a React.PropTypes validation function for the `transformMatrix` prop on `View`. This is deprecated and will not work in the next major version. You may be seeing this warning due to a third-party PropTypes library. See https://fb.me/react-warning-dont-call-proptypes for details.

...

After reading the link they provided (https://fb.me/react-warning-dont-call-proptypes) and looking at some code, it seems like the way styles and transforms are done do violate their new rules. I'm not sure how to best fix it.

Any ideas?

RealOrangeOne commented 8 years ago

what component are you trying to test. Were you able to find examples in our library that call them or is it in some of our dependencies.

jasonfma commented 8 years ago

This warning is new with RN 0.32.0 - specifically React 15.3.0.

Here's a sample test that is only using a View:

import React from 'react';
import {
  View,
} from 'react-native';
import { expect } from 'chai';
import { shallow } from 'enzyme';

describe.only('test spec', () => {
  it('should work', () => {
    const wrapper = shallow(<View style={{ height: 30 }} />);
    expect(wrapper).to.exist;
  });
});

with warning


Warning: You are manually calling a React.PropTypes validation function for the `style` prop on `View`. This is deprecated and will not work in the next major version. You may be seeing this warning due to a third-party PropTypes library. See https://fb.me/react-warning-dont-call-proptypes for details.
Warning: You are manually calling a React.PropTypes validation function for the `transformMatrix` prop on `View`. This is deprecated and will not work in the next major version. You may be seeing this warning due to a third-party PropTypes library. See https://fb.me/react-warning-dont-call-proptypes for details.
RealOrangeOne commented 8 years ago

I'm not sure what the problem is with that. The View component doesnt do anything. I fear it's in one of our dependancies

jasonfma commented 8 years ago

I think the culprit is in StyleSheetPropType.js


function StyleSheetPropType(shape) {
  const shapePropType = PropTypes.shape(shape);   <-------- here
  return function (props, propName, componentName, location) {
    let newProps = props;
    if (props[propName]) {
      // Just make a dummy prop object with only the flattened style
      newProps = {};
      newProps[propName] = flattenStyle(props[propName]);
    }
    return shapePropType(newProps, propName, componentName, location);  <-------- used here
  };
}

This seems like it's the part that's violating the new rule https://facebook.github.io/react/warnings/dont-call-proptypes.html#dont-call-proptypes-directly

var apiShape = React.PropTypes.shape({
  body: React.PropTypes.object,
  statusCode: React.PropTypes.number.isRequired
}).isRequired;

// Not supported!
var error = apiShape(json, 'response');

I think similarly in TransformPropTypes.js.

I'd love to help with a fix but I'm not sure what's actually going on here and what the best remediation would be.

RealOrangeOne commented 8 years ago

ah, good to know. Yes that does look like the culprit. No, i'm not sure what the best solution is either. I think we may need to look at how proptypes are generated in the first place, and any other libraries that define custom ones to work out the nicest way of doing this

jasonfma commented 8 years ago

Do we need to do whatever custom proptype stuff we're doing here? What was the original intent behind flattening the styles here?

RealOrangeOne commented 8 years ago

I'm not actually sure. I didnt write that bit. Maybe a question for @lelandrichardson

jasonfma commented 8 years ago

This does prevent us from upgrading to the latest RN since we prefer to treat all warnings (especially proptype warnings) as errors in our tests. I'd be happy to dedicate some time to this to figure out a solution and submit a PR. @lelandrichardson can you provide more context into why the styles and transform proptype mocks were implemented this way?

evollu commented 8 years ago

I changed location or ...rest and everything is working now

jasonfma commented 7 years ago

Any update on an expected release @RealOrangeOne ? My xcode just automatically updated and I need this to upgrade to RN 0.33. If not I can just install the old xcode. Thanks :)