Make react-intl react 16 compatible

[ptomasroos]

Closes #1078
This is not a breaking change since the access of the ref is used when calling getWrappedInstance()

In order to support React 16. We need to remove the legacy way of using ref's by setting a string.
This is now marked as legacy.

We advise against it because string refs have some issues, are considered legacy, and are likely to be removed in one of the future releases. 

Note

If you’re currently using this.refs.textInput to access refs, we recommend using either the callback pattern or the createRef API.

https://reactjs.org/docs/refs-and-the-dom.html#legacy-api-string-refs

[ptomasroos]

As mentioned by @ericf in this usage how to access the ref from the injectIntl component.
#811 (comment)

And thats how we use it through out our web app's as well.

[ptomasroos]

Tests are failing because of current state in master i would say?

Jest: Coverage for functions (98.73%) does not meet global threshold (100%)

[redonkulus]

Thanks for the PR. I'm fine merging if it doesn't break anyone.

As for the failing build, it looks like one of the tests are failing:

 FAIL  test/unit/react-intl-with-locales.js
  ● Test suite failed to run
    /home/travis/build/yahoo/react-intl/src/utils.js:2
    }});
    ^
    
    SyntaxError: Unexpected token }
      
      at new Script (vm.js:79:7)
      at Object.<anonymous> (src/inject.js:20:572)

It's not related to your PR, since master has the same problem, but do you mind seeing if it's an easy fix?

[ptomasroos]

Sadly i can't reproduce this locally with same node version and can't really find the cause for this.
Seems to happen only on node 10 for some reason?

[ptomasroos]

Any way to retrigger the test @redonkulus or continue to merge this anyway?

[redonkulus]

So the unit tests pass but the coverage is below 100%. master is 100% for all functions and those builds are passing. Can you get the inject file back up to 100% coverage?

[ptomasroos]

TBH looking at the tests it look seriously solid. @redonkulus

[ptomasroos]

But i will take a look if I can find something

[ptomasroos]

Sorry @redonkulus I cant bring this to 100, its complaining about the ref callback assignment and I can't really find a way to test that "logic". Can you give me some insights?

[redonkulus]

@ptomasroos A quick googling found this answer for testing the ref: https://stackoverflow.com/questions/48088489/react-jestjs-enzyme-how-to-test-for-reference-function

Another option you can do for coverage sake is just have jest ignore that line.

[ptomasroos]

Thanks for the patience @redonkulus.
I gave it a try through several examples out on the web, yours as well. But couldn't make it to get covered.

Hence I added a ignore line instead since I already feel this is covered through the existing lines which check the ref assignment through the invariant checks.

Would be great if we could merge and release this in order to avoid splitting the community into branches and forks as currently 16.6 users need to use https://www.npmjs.com/package/@freedombase/react-intl

[redonkulus]

Thanks for doing the work here!

[redonkulus]

published in v2.7.2