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-v16] Part I: Basics #662

Merged
merged 6 commits into from
Jul 31, 2018

Conversation

bchrobot
Copy link
Contributor

@bchrobot bchrobot commented Jun 20, 2018

This has just the bare bones, no complications dependency updates. This breaks up PR #627 into 3 sub-PRs. Because of various breaking changes, I believe Spoke will not run until they are all updated in lockstep, but at least the functionality changes are broken up into more easily reviewed segments:

The above PRs are against the branch for this PR.

@shakalee14 shakalee14 added the S-work in progress (WIP) Status: PR label for work that is not yet ready to be reviewed label Jul 6, 2018
@bchrobot
Copy link
Contributor Author

bchrobot commented Jul 7, 2018

I have made decent progress on this. Things I am stuck on / would like feedback before continuing are:

  • How to handle the removal of the react-router's match() utility method used for server-side rendering. See my comment here.
  • Sanity check on webpack upgrade. Documentation is mixed. See that PR.
  • General react-apollo plumbing (see that commit).
  • Rewrite of loadData HOC (see that commit).

@bchrobot bchrobot changed the base branch from main to react-v16 July 26, 2018 16:48
@bchrobot bchrobot changed the title [WIP] React v16 upgrade [react-v16] Part I: Basics Jul 26, 2018
Copy link
Collaborator

@schuyler1d schuyler1d left a comment

Choose a reason for hiding this comment

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

this is fine to go into react-v16 branch.

Have you tested that the onClick works on mobile devices? I believe we moved away from click specifically because of an issue with some mobile devices.

@bchrobot
Copy link
Contributor Author

I have not tested it on mobile yet. But will as soon as there is a running version of the v16 branch.

react-tap-event-plugin is officially deprecated though:

React 16.4 removes a lot of internals (#121) this plugin depends on and will break the plugin.

Since the problem it solves has been fixed in most browsers by now you should migrate away from this plugin.

-- https://github.com/zilverline/react-tap-event-plugin#deprecated

@schuyler1d
Copy link
Collaborator

For the match stuff in router, I would just skip the server-side stuff. just render the client all the time, and then let's have the client say it's wrong. (That can be a separate PR separate from react-v16 just simplifying it.)

@bchrobot
Copy link
Contributor Author

@schuyler1d schuyler1d merged commit cc81b7a into StateVoicesNational:react-v16 Jul 31, 2018
codygordon pushed a commit that referenced this pull request Jul 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-work in progress (WIP) Status: PR label for work that is not yet ready to be reviewed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants