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

React 16 #512

Merged
merged 51 commits into from
Mar 5, 2020
Merged

React 16 #512

merged 51 commits into from
Mar 5, 2020

Conversation

mjoyce91
Copy link

No description provided.

@mjoyce91
Copy link
Author

We'll have to refactor most of our functional components:

https://airbnb.io/enzyme/docs/api/ShallowWrapper/props.html#example

See dd68b42 for an example - we no longer have access to unused props, but we can check props on the root element.

@@ -35,6 +36,8 @@ class Editor extends Component {
submitProps={{ id: SUBMIT_BUTTON_ID }}
/>
<div className="usa-grid-full markdown-editor">
{
/*
<MarkdownEditor
Copy link
Author

@mjoyce91 mjoyce91 Oct 26, 2019

Choose a reason for hiding this comment

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

Even https://github.com/jrm2k6/react-markdown-editor/tree/1.0.0 breaks, so we'll need to look into replacing this library

Just needed to remove the styles.

@@ -12,7 +12,7 @@ import FA from 'react-fontawesome';
import { addHours, format, parse, subHours } from 'date-fns';
import DayPickerInput from 'react-day-picker/DayPickerInput';
import { DateUtils } from 'react-day-picker';
import TimeInput from 'time-input';
// import TimeInput from 'time-input';
Copy link
Author

@mjoyce91 mjoyce91 Oct 26, 2019

Choose a reason for hiding this comment

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

This library is no longer maintained, but this looks straight forward to fork into our repo

https://github.com/alanclarke/time-input/blob/master/src/TimeInput.js#L13

This fork is React 16 compatible - https://github.com/radumardale/react-keyboard-time-input

@@ -15,11 +15,13 @@ exports[`Alert displays error alert with multiple messages 1`] = `
</h3>
<p
className="usa-alert-text"
key="ZG9_0yeXR"
Copy link
Author

@mjoyce91 mjoyce91 Oct 26, 2019

Choose a reason for hiding this comment

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

Need to ignore or mock shortid keys Done

@mjoyce91
Copy link
Author

mjoyce91 commented Oct 28, 2019

From https://github.com/cerner/terra-core/wiki/React-16-Migration-Guide#noted-changes

In React 16, HTML attributes are more strict.
Before, if you had a boolean attribute on an HTML element, it would convert a JavaScript boolean value to a string value. With React 16, you must pass a string value.
With React 15, you could set aria-require={false}.
With React 16, you would want to set that as aria-required="false"

See 8d2abaa

@mjoyce91
Copy link
Author

mjoyce91 commented Oct 30, 2019

Optional upgrades:

react-countup - breaks at 4.2.2, probably just new syntax/config
react-autosuggest - on latest
react-day-picker - on latest
react-helmet - on latest
react-linkify - on latest stable
react-loadable - on latest
react-loading-skeleton - works, but not sure if we’re using correctly (loading state?)
react-markdown - on latest
react-picky - on latest
react-responsive - upgraded to latest, works
react-router
react-router-dom
react-router-redux
react-router-scroll-4
react-scroll - on latest
react-scroll-up-button - on latest
react-scrollbar - on latest
react-simple-dropdown - on latest
react-spring - breaks at 8.0.27 'react-spring' does not contain an export named 'Spring'.
react-tippy - upgraded to 1.3.1, probably not React 16-related
react-toastify - upgraded to 5.4.0, probably not React 16-related
redux
redux-persist - upgraded to 6.0.0, but not sure if we’re using correctly for search sorts
redux-saga - 1.0.5 - Cannot find module 'redux-saga/lib/internal/sagaHelpers' from 'index.js'
redux-thunk - 2.2 -> 2.3 is just typescript updates

@mjoyce91
Copy link
Author

👀 @scott062

history: PropTypes.shape({}).isRequired,
history: PropTypes.shape({
listen: PropTypes.func,
location: PropTypes.object,
Copy link
Author

Choose a reason for hiding this comment

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

Should use shape instead of object

@@ -44,7 +44,7 @@
"indent": ["error", 2, { "ignoredNodes": ["JSXAttribute", "JSXSpreadAttribute"], "SwitchCase": 1 }],
"react/jsx-indent": [0],
"react/jsx-indent-props": ["error", 2],
"complexity": ["warn", { "max": 20 }],
"complexity": ["warn", { "max": 30 }],
Copy link
Author

Choose a reason for hiding this comment

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

3️⃣ 0️⃣

@@ -94,7 +94,7 @@ ResultsCondensedCardBottom.contextTypes = {
};

ResultsCondensedCardBottom.propTypes = {
position: POSITION_DETAILS.isRequired,
position: PropTypes.oneOf([POSITION_DETAILS, { position: POSITION_DETAILS }]).isRequired,
Copy link
Author

Choose a reason for hiding this comment

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

👍

Copy link
Author

Choose a reason for hiding this comment

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

Ones like this will actually need to be oneOfType. I'll fix

@mjoyce91
Copy link
Author

mjoyce91 commented Mar 3, 2020

Thanks @scott062 ! Looks good

mjoyce91 and others added 7 commits March 4, 2020 11:03
* dev:
  Bump helmet from 3.21.2 to 3.21.3
  Update test since this reducer now spreads
  Use new dedicated routes to get just the IDs of favorite positions; don't refetch all userProfile data on favorite add/delete
  Remove uneccessary styling
  Fix it for real
  Fix css scope
  Media query nested to test for ie11
  Fix flex-basis and min-width
  4 -> 3
  Disable runInBand but limit maxWorkers to 4
…te-TalentMAP into update/react-16

* 'update/react-16' of github.com:MetaPhase-Consulting/State-TalentMAP:
  Get default sort value from Constants/Sort
  fix: default sorting for client list to last_name
@mjoyce91
Copy link
Author

mjoyce91 commented Mar 4, 2020

@rtirserio @scott062 @elizabeth-jimenez - can you review this? You don't need to go through all 5,000 lines - mainly concerned with functionality and local dev environments. Also, not concerned about warning logs (mainly around lifecycle methods) right now.

You'll need to:

  • Install node >=12
  • Restart your IDE to get the new eslint rules to catch
  • yarn install --force - you may need to clear out your node_modules if you run into issues

@mjoyce91 mjoyce91 removed WIP Work in progress do not merge labels Mar 5, 2020
* dev:
  Update snapshots
  Add select all field to cdo search
@mjoyce91
Copy link
Author

mjoyce91 commented Mar 5, 2020

I'm going to need to upgrade this to get hooks to work with hot loader - https://github.com/gaearon/react-hot-loader Never mind, was using hooks incorrectly!

Copy link

@scott062 scott062 left a comment

Choose a reason for hiding this comment

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

Checked basic bidder functionality
Checked basic CDO functionality
Checked admin functionality

👍 Looks great

@mjoyce91 mjoyce91 merged commit 3216363 into dev Mar 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants