Update Codebase to use React 16 - DO NOT MERGE #1175

JakeLaCombe · 2018-01-24T22:36:00Z

Resolves #658

Testing Branch for SD's to test terra core against.

You can find React 16 here: https://terra-core-deployed-pr-1175.herokuapp.com/static/#/site

ghfilter

.*.md

StephenEsser · 2018-01-24T22:56:13Z

JakeLaCombe · 2018-01-25T00:45:41Z

@StephenEsser more to come as well! Almost all the wdio snapshots need to be updated :O

mhemesath · 2018-01-25T14:34:57Z

We need to fix all the screenshots that were just on the element and now are full page. Those waste size and less sensitive to visual diffs.

bjankord · 2018-01-25T22:59:33Z

@mhemesath Yeah I'm gonna sync with Jake with he's back in the office to get these updated. Should cut the file diff down a ton.

bjankord · 2018-01-25T23:29:40Z

Seeing a few console errors on different pages. Mainly just minor prop-type stuff.

https://terra-core-deployed-pr-1175.herokuapp.com/static/#/site/date-time-picker

Error

warning.js:33 Warning: Failed prop type: Invalid prop `selected` of type `string` supplied to `DatePicker`, expected `object`.

https://terra-core-deployed-pr-1175.herokuapp.com/static/#/site/embedded-content-consumer

Error

warning.js:33 Warning: Received `true` for a non-boolean attribute `fill`.
If you want to write it to the DOM, pass a string instead: fill="true" or fill={value.toString()}.

Error

warning.js:33 Warning: Each child in an array or iterator should have a unique "key" prop.

Check the render method of `PropsTable`. See https://fb.me/react-warning-keys for more information.

https://terra-core-deployed-pr-1175.herokuapp.com/static/#/site/form-radio

Error

Warning: React does not recognize the `isDisabled` prop on a DOM element. 
If you intentionally want it to appear in the DOM as a custom attribute, spell it as lowercase 
`isdisabled` instead. If you accidentally passed it from a parent component, remove it from the 
DOM element.

https://terra-core-deployed-pr-1175.herokuapp.com/static/#/site/image

Error

warning.js:33 Warning: React does not recognize the `placeholderSrc` prop on a DOM element. If you intentionally want it to appear in the DOM as a custom attribute, spell it as lowercase `placeholdersrc` instead. If you accidentally passed it from a parent component, remove it from the DOM element.

bjankord · 2018-01-26T19:50:28Z

Need to look into the branch deployment for this. It is currently failing out when trying to run webpack. It looks like its calling webpack before maybe npm install has run?

 > webpack --config webpack.prod.config --progress
       
module.js:538
    throw err;
    ^
Error: Cannot find module 'postcss-assets-webpack-plugin'

emilyrohrbough · 2018-01-30T22:17:34Z

packages/terra-date-picker/tests/nightwatch/components/DatePickerDefault.jsx

-    <DatePicker
-      name="date-input"
-    />
+    <div>


Oops. I think something happened here 😅

Totally forgot about this!

I had issues using popper, and so I created a large scroll page to test scrolling with popper. This can all be undone.

emilyrohrbough · 2018-01-30T22:18:31Z

packages/terra-date-picker/tests/nightwatch/date-picker-spec.js

@@ -139,7 +139,7 @@ module.exports = resizeTo(['tiny', 'huge'], {

    browser.expect.element('.react-datepicker').to.be.present;

-    browser.click('.react-datepicker__today-button');
+    browser.click('[aria-label="day-10"]');


Is this mapped to the same button?

emilyrohrbough · 2018-01-30T22:20:08Z

packages/terra-demographics-banner/src/DemographicsBanner.jsx

@@ -67,6 +67,7 @@ const defaultProps = {
 const contextTypes = {
  /* eslint-disable consistent-return */
  intl: (context) => {
+    console.log('Intl Checking');


Needs removed

emilyrohrbough · 2018-01-30T22:20:24Z

packages/terra-demographics-banner/tests/jest/DemographicsBanner.test.jsx

  const banner = shallow(<IntlProvider locale={locale} messages={messages}><DemographicsBanner /></IntlProvider>);
  expect(banner).toMatchSnapshot();
 });

-it('renders the banner wrapper with all props', () => {
+xit('renders the banner wrapper with all props', () => {


emilyrohrbough · 2018-01-30T22:20:29Z

packages/terra-demographics-banner/tests/jest/DemographicsBanner.test.jsx

@@ -8,12 +8,12 @@ import messages from '../../translations/en-US.json';

 const locale = 'en-US';

-it('renders a blank banner wrapper', () => {
+xit('renders a blank banner wrapper', () => {


emilyrohrbough · 2018-01-30T22:24:20Z

packages/terra-heading/tests/jest/Heading.test.jsx

@@ -51,12 +51,12 @@ describe('Heading', () => {
  // Prop Tests
  it('should have all props including customProps set correctly', () => {
    const heading = shallow(<Heading level={1} id="id" size="small" color="#f00" weight={200} isItalic isVisuallyHidden>All props and custom attrs</Heading>);


Could we add a snapshot test here instead? All of this could be removed then

emilyrohrbough · 2018-01-30T22:24:33Z

packages/terra-hookshot/tests/jest/Hookshot.test.jsx

@@ -7,7 +7,7 @@ it('should shallow a default component', () => {
  expect(hookshot).toMatchSnapshot();
 });

-it('should render a default component', () => {
+xit('should render a default component', () => {


emilyrohrbough · 2018-01-30T22:25:17Z

packages/terra-icon/src/IconBase.jsx

@@ -81,7 +81,7 @@ const IconBase = ({

  attributes.height = height;
  attributes.width = width;
-  attributes.focusable = focusable;
+  attributes.focusable = focusable.toString();


What did we make this change?

emilyrohrbough · 2018-01-30T22:25:36Z

packages/terra-legacy-theme/CHANGELOG.md

@@ -3,6 +3,8 @@ Changelog

 Unreleased
 ----------
+### Changed


This isn't needed

emilyrohrbough · 2018-01-30T22:25:58Z

packages/terra-mixins/CHANGELOG.md

@@ -3,6 +3,8 @@ Changelog

 Unreleased
 ----------
+### Changed


This isn't needed

emilyrohrbough · 2018-01-30T22:26:12Z

packages/terra-modal/tests/jest/Modal.test.jsx

@@ -11,7 +11,7 @@ it('should mount an open modal', () => {
  expect(modal).toMatchSnapshot();
 });

-it('should render an open modal', () => {
+xit('should render an open modal', () => {


emilyrohrbough · 2018-01-30T22:26:33Z

packages/terra-popup/tests/jest/Popup.test.jsx

@@ -7,7 +7,7 @@ it('should shallow a default component', () => {
  expect(popup).toMatchSnapshot();
 });

-it('should render a default component', () => {
+xit('should render a default component', () => {


emilyrohrbough · 2018-01-30T22:26:53Z

packages/terra-popup/tests/nightwatch/components/OverlayPopup.jsx

@@ -15,7 +15,6 @@ class DefaultPopup extends React.Component {
    this.overlayStyle = document.documentElement.style.overflow;
    this.overlayId = document.documentElement.id;
    document.documentElement.id = 'popup-overlay-test';
-    document.documentElement.style.overflow = 'auto';


Why was this removed?

emilyrohrbough · 2018-01-30T22:27:28Z

packages/terra-site/package.json

@@ -114,8 +114,9 @@
    "postcss-loader": "^2.0.6",
    "postcss-rtl": "^1.1.2",
    "raw-loader": "^0.5.1",
-    "react": "^15.4.2",
-    "react-dom": "^15.4.2",
+    "raf": "^3.4.0",


What is raf?

emilyrohrbough · 2018-01-30T22:28:17Z

packages/terra-site/webpack.config.js

@@ -23,6 +23,7 @@ const threadLoaderRule = {

 module.exports = {
  entry: {
+    raf: 'raf/polyfill',


Why are we using two polyfills now? Will this be required by all consumers?

React 16 requires request animation frame. https://reactjs.org/blog/2017/09/26/react-v16.0.html

emilyrohrbough · 2018-01-30T22:29:01Z

packages/terra-status/tests/jest/Status.test.jsx

@@ -38,9 +38,9 @@ it('should render a arrange with status and customProps', () => {
 it('should have all props including customProps set correctly', () => {
  const arrangeWithStatus = <Status color="green" id="id" >{arrange}</Status>;
  const wrapper = shallow(arrangeWithStatus);


I think we can update this to a screenshot & jest will do this comparison for us

emilyrohrbough · 2018-01-30T22:30:42Z

packages/terra-time-input/tests/nightwatch/component/TimeInputTwelveHourMobile.jsx

@@ -18,12 +18,6 @@ export default class TimeInputElement extends React.Component {
    }
  }

-  componentWillUnmount() {


Is this a 16 change? Should this be moved else where?

emilyrohrbough · 2018-01-30T22:31:10Z

packages/terra-toggle/tests/jest/Toggle.test.jsx

@@ -21,9 +21,11 @@ describe('Toggle', () => {
  // Prop Tests
  it('should have all props set correctly', () => {
    const toggle = shallow(<Toggle isAnimated isOpen>All props</Toggle>);


I think we can update this to a screenshot & jest will do this comparison for us

emilyrohrbough · 2018-01-30T22:33:21Z

We shouldn't need to check in the DEPENDENCIES.md files. That'll get updated during postpublish

terra-bot · 2018-01-31T02:49:42Z

	Warnings
⚠️	❗ Big PR. Consider breaking this into smaller PRs if applicaple

Generated by 🚫 dangerJS

JakeLaCombe · 2018-01-31T17:05:35Z

I'll close this soon and put out a more throughouh review. I wanted a link for functional testing, and also wanted to test the Travis CI builds. I'll be sure to address the comments in this review.

JakeLaCombe · 2018-01-31T19:50:36Z

Closing. I think I have enough info to verify my next pull request will operate. I'll make sure all comments here are addressed.

Update Codebase to use React 16

39c176a

bjankord temporarily deployed to terra-core-deployed-pr-1175 January 24, 2018 22:36 Inactive

Fix wdio tests

95decab

Add Test Container to wrap wdio tests

d85cb58

JakeLaCombe had a problem deploying to terra-core-deployed-pr-1175 January 25, 2018 06:11 Failure

emilyrohrbough added the do not merge label Jan 25, 2018

bjankord closed this Jan 25, 2018

bjankord reopened this Jan 25, 2018

bjankord had a problem deploying to terra-core-deployed-pr-1175 January 25, 2018 23:30 Failure

bjankord had a problem deploying to terra-core-deployed-pr-1175 January 26, 2018 19:16 Failure

bjankord had a problem deploying to terra-core-deployed-pr-1175 January 26, 2018 19:32 Failure

Jake LaCombe added 3 commits January 29, 2018 19:37

Merge branch 'master' into React-16-Upgrade

90d366c

Fix First Nightwatch Tests

061b4bd

Merge branch 'master' into React-16-Upgrade

4a03eb9

emilyrohrbough reviewed Jan 30, 2018

View reviewed changes

packages/terra-legacy-theme/CHANGELOG.md

@@ -3,6 +3,8 @@ Changelog

Unreleased

----------

### Changed

Copy link

Contributor

emilyrohrbough Jan 30, 2018

Choose a reason for hiding this comment

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

This isn't needed

emilyrohrbough reviewed Jan 30, 2018

View reviewed changes

packages/terra-mixins/CHANGELOG.md

@@ -3,6 +3,8 @@ Changelog

Unreleased

----------

### Changed

Copy link

Contributor

emilyrohrbough Jan 30, 2018

Choose a reason for hiding this comment

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

This isn't needed

emilyrohrbough reviewed Jan 30, 2018

View reviewed changes

bjankord temporarily deployed to terra-core-deployed-pr-1175 January 31, 2018 02:33 Inactive

Merge branch 'master' into React-16-Upgrade

a42d245

bjankord temporarily deployed to terra-core-deployed-pr-1175 January 31, 2018 02:43 Inactive

Jake LaCombe added 3 commits January 31, 2018 11:04

Fix Tabs

a90fe79

Fix Lint

0696195

Fix Jest tests

d680ca0

JakeLaCombe deployed to terra-core-deployed-pr-1175 January 31, 2018 17:05 View deployment

JakeLaCombe closed this Jan 31, 2018

bjankord deleted the React-16-Upgrade branch February 27, 2018 20:35

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update Codebase to use React 16 - DO NOT MERGE #1175

Update Codebase to use React 16 - DO NOT MERGE #1175

JakeLaCombe commented Jan 24, 2018 •

edited by bjankord

Loading

StephenEsser commented Jan 24, 2018

JakeLaCombe commented Jan 25, 2018

mhemesath commented Jan 25, 2018

bjankord commented Jan 25, 2018 •

edited

Loading

bjankord commented Jan 25, 2018 •

edited

Loading

bjankord commented Jan 26, 2018

emilyrohrbough Jan 30, 2018 •

edited

Loading

JakeLaCombe Jan 31, 2018

emilyrohrbough Jan 30, 2018

emilyrohrbough Jan 30, 2018

emilyrohrbough Jan 30, 2018 •

edited

Loading

emilyrohrbough Jan 30, 2018 •

edited

Loading

emilyrohrbough Jan 30, 2018

emilyrohrbough Jan 30, 2018 •

edited

Loading

emilyrohrbough Jan 30, 2018

emilyrohrbough Jan 30, 2018

emilyrohrbough Jan 30, 2018

emilyrohrbough Jan 30, 2018

emilyrohrbough Jan 30, 2018

emilyrohrbough Jan 30, 2018

emilyrohrbough Jan 30, 2018

emilyrohrbough Jan 30, 2018

JakeLaCombe Jan 31, 2018

emilyrohrbough Jan 30, 2018

emilyrohrbough Jan 30, 2018

emilyrohrbough Jan 30, 2018

emilyrohrbough commented Jan 30, 2018

terra-bot commented Jan 31, 2018 •

edited

Loading

JakeLaCombe commented Jan 31, 2018

JakeLaCombe commented Jan 31, 2018

Update Codebase to use React 16 - DO NOT MERGE #1175

Update Codebase to use React 16 - DO NOT MERGE #1175

Conversation

JakeLaCombe commented Jan 24, 2018 • edited by bjankord Loading

StephenEsser commented Jan 24, 2018

JakeLaCombe commented Jan 25, 2018

mhemesath commented Jan 25, 2018

bjankord commented Jan 25, 2018 • edited Loading

bjankord commented Jan 25, 2018 • edited Loading

bjankord commented Jan 26, 2018

emilyrohrbough Jan 30, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

emilyrohrbough Jan 30, 2018 • edited Loading

Choose a reason for hiding this comment

emilyrohrbough Jan 30, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

emilyrohrbough Jan 30, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

emilyrohrbough commented Jan 30, 2018

terra-bot commented Jan 31, 2018 • edited Loading

JakeLaCombe commented Jan 31, 2018

JakeLaCombe commented Jan 31, 2018

JakeLaCombe commented Jan 24, 2018 •

edited by bjankord

Loading

bjankord commented Jan 25, 2018 •

edited

Loading

bjankord commented Jan 25, 2018 •

edited

Loading

emilyrohrbough Jan 30, 2018 •

edited

Loading

emilyrohrbough Jan 30, 2018 •

edited

Loading

emilyrohrbough Jan 30, 2018 •

edited

Loading

emilyrohrbough Jan 30, 2018 •

edited

Loading

terra-bot commented Jan 31, 2018 •

edited

Loading