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

got warnings while upgraded to react v15.0 rc #114

Closed
kalaxiya opened this issue Mar 24, 2016 · 14 comments
Closed

got warnings while upgraded to react v15.0 rc #114

kalaxiya opened this issue Mar 24, 2016 · 14 comments

Comments

@kalaxiya
Copy link

react added a helpful warning in v15.0 rc-1:

React DOM: When specifying a unit-less CSS value as a string, a future version will not add px automatically. This version now warns in this case (ex: writing style={{width: '300'}}. (Unitless number values like width: 300 are unchanged.)

warnings like these:

Warning: adivtag (owner:Saturation) was passed a numeric string value for CSS propertybottom(value:0) which will be treated as a unitless number in a future version of React.

Warning: adivtag (owner:Alpha) was passed a numeric string value for CSS propertyright(value:0) which will be treated as a unitless number in a future version of React.

@genert
Copy link

genert commented Apr 1, 2016

Yes, it would be good idea to change unitless values to 'px' values to avoid errors in future release of React.

However, I suggest to use 0.14.8 until RC finalizes. ;)

@Kerumen
Copy link

Kerumen commented Apr 13, 2016

React 15 is out now 🎉

@jaeh
Copy link
Contributor

jaeh commented Apr 13, 2016

i currently am in the process of upgrading all dependencies and fixing the issues that got caused in my codebase when updating to 15.0.1,
@casesandberg are you interested in a contribution of these updates and changes or are you working on those too?

@casesandberg
Copy link
Owner

Would love your help getting this upgraded @jaeh!

@jaeh
Copy link
Contributor

jaeh commented Apr 17, 2016

#132 fixes all errors in my codebase,
unfortunately i can not get 3 tests to pass,
this happens both in my fork and if i directly clone your repo.

  Compact
Warning: a `div` tag (owner: `Raised`) was passed a numeric string value for CSS property `top` (value: `0`) which will be treated as a unitless number in a future version of React.
Warning: a `div` tag (owner: `Raised`) was passed a numeric string value for CSS property `right` (value: `0`) which will be treated as a unitless number in a future version of React.
Warning: a `div` tag (owner: `Raised`) was passed a numeric string value for CSS property `bottom` (value: `0`) which will be treated as a unitless number in a future version of React.
Warning: a `div` tag (owner: `Raised`) was passed a numeric string value for CSS property `left` (value: `0`) which will be treated as a unitless number in a future version of React.
    1) should be able to take custom swatches
    ✓ should pass up data on change

  Swatches
    ✓ should pass up data onChange (142ms)
    2) should render SwatchesGoup for each top-level array passed to props.colors

  SwatchesColor
    ✓ should pass the color back up when clicked

  SwatchesGroup
    ✓ should pass up data onClick
    3) should render SwatchesGroupGoup for each top-level array passed to props.colors

is this expected or caused by the positional values erroring?
i also can not find where those get set?

@jaeh jaeh mentioned this issue Apr 17, 2016
@islemaster
Copy link
Contributor

islemaster commented May 2, 2016

@jaeh Is this the error you're seeing?

  1) Compact should be able to take custom swatches:
     TypeError: Cannot read property '_renderedChildren' of undefined
      at Context.<anonymous> (Compact.test.js:17:18)

I've got that test passing by switching to ReactDOM utilities for inspecting the DOM state. Right now that test is digging into private component properties that may not exist anymore in React 15+.

    const CompactComponent = TestUtils.renderIntoDocument(<Compact {...props} colors={['#fff', '#999', '#222']} />)
    let colors = CompactComponent.refs.colors._reactInternalComponent._renderedChildren
    let colorCount = 0
    for (var color in colors) {
      colorCount += 1
    }
    expect(CompactComponent.props.colors).to.exist
    expect(colorCount).to.equal(4)

Instead, I've done the following. I'm still getting the numeric string value warnings, but the test is passing. I think it's in keeping with the spirit of the test, without depending on React internals.

    const CompactComponent = TestUtils.renderIntoDocument(<Compact {...props} colors={['#fff', '#999', '#222']} />)
    let colorCount = ReactDOM.findDOMNode(CompactComponent.refs.colors).children.length
    expect(CompactComponent.props.colors).to.exist
    expect(colorCount).to.equal(4)

I've got tests passing and warnings cleared in code-dot-org#1. Not sure if I've missed anything though.

@casesandberg
Copy link
Owner

Sorry all, this has been fixed in 2.1.0

@islemaster
Copy link
Contributor

Thanks!

@willsoto
Copy link

willsoto commented May 7, 2016

I am still seeing this issue.

React: v15.0.2
React Color: v2.1.0
React DOM: v15.0.2

Anyone have any suggestions?

@islemaster
Copy link
Contributor

It sounds like those warnings are a result of Absolute: '0 0 0 0' style rules like this one: https://github.com/casesandberg/react-color/blob/master/modules/react-material-design/src/components/Raised.js#L19.

This (the numeric string value warning) still repro's for me on a clean clone of master by running npm install && npm install mocha && npm test in the "Compact should be able to take custom swatches" test (It appears mocha might be a missing devDependency of this package).

I believe I have those warnings fixed on our fork as of code-dot-org#1 and was going to open a pull request as soon as we've seen our application pass its test suite using the change.

@casesandberg
Copy link
Owner

casesandberg commented May 7, 2016

🙌

@islemaster
Copy link
Contributor

Timing update: Code.org is likely to ship with our React 15 changes today. I'll open the PR with our changes this evening.

@wzup
Copy link

wzup commented Jun 10, 2016

Absolutely useless warning. It doesn't show the source of the issue.

@kennjaramilla
Copy link

Issue resolved. Thanks guys!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

9 participants