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

Preparations for React 17 compatibility, fix master build #341

Merged
merged 6 commits into from
Jan 6, 2021

Conversation

sourcegraph-wk
Copy link

@sourcegraph-wk sourcegraph-wk commented Dec 15, 2020

These changes / builds will first be reviewed by a member of the Client Platform team.

Repo owners will be notified when it is ready for additional review, merge and release.

Motivation

React 17 was released at the end of October and came with some exciting benefits. For the full list, see the ReactJS blog post.

For Workiva, upgrading so promptly is even more exciting because it:

  1. supports our Material-UI (MUI) effort. MUI will support React 17 in their next major release. This major is the one that Client Platform will use as the corner stone for the MUI effort. Therefore, to be compatible, Workiva should adopt React 17 as well.
  2. helps with the Microfrontends effort. This is primarily because React 17 updated how event delegation works and made it safer to embed React trees within each other. These changes, along with others, provide more options when implementing a Microfrontends architecture by making React more compatible with the ShadowDOM.

Wdesk is currently using ReactJS 16.13.1, and the Client Platform team has been working to upgrade our Dart wrappers to be compatible with the latest major version of ReactJS (17.x). Now that major releases of the react (6.0.0) and over_react (4.0.0) packages are ready to release, its time to begin making the entire Workiva front-end ecosystem compatible!

Changes

This upgrade will be fundamentally simpler than the React 16 upgrade and will be much easier in comparison.

All changes/fixes will be handled by Client Platform before repo owners are requested for review(s).

  1. Raise the upper bounds of the react / over_react dependencies to allow the new major release versions to be pulled in once all downstream dependencies are also compatible.
  2. Fix any CI failures.
  3. Find and address any other code affected by breaking changes or known edge cases, which may not have been caught by CI.

Additional changes

  • Add Chrome 86 DDC workaround to fix master CI
  • Update some dev deps
  • Fix some deprecation warnings and misc hints

Release Notes

Make this library compatible with ReactJS 17.

QA Instructions

React 17 Testing PR

  • Verify all breaking changes to be addressed have been checked off in the Changes section
  • Using the React 17 testing PR also made into this repo:
    • Verify CI passed
    • Manually test areas without functional test coverage.

References

More information about React 17

The 6.0.0 react-dart release

The 4.0.0 over_react release

Created by Sourcegraph campaign Workiva/react_17_rollout.

@aviary-wf
Copy link

Security Insights

No security relevant content was detected by automated scans.

Action Items

  • Review PR for security impact; comment "security review required" if needed or unsure
  • Verify aviary.yaml coverage of security relevant code

Questions or Comments? Reach out on Slack: #support-infosec.

@sourcegraph-wk sourcegraph-wk changed the title (CPLAT-12478) Preparations for React 17 compatibility Preparations for React 17 compatibility Dec 15, 2020
import 'dart:html';

import 'package:over_react/components.dart' show ErrorBoundary;
Copy link
Contributor

Choose a reason for hiding this comment

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

I fixed some deprecation warnings for ErrorBoundary/setClientConfiguration while I was in here

@@ -21,7 +21,6 @@ import 'package:http_parser/http_parser.dart' show MediaType;
import 'package:mockito/mockito.dart';
import 'package:test/test.dart';
import 'package:w_transport/mock.dart';
import 'package:w_transport/src/http/auto_retry.dart';
Copy link
Contributor

Choose a reason for hiding this comment

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

Unused import

<head>
<title>{{testName}} Test</title>
<script>
// Workaround to Chrome 86 DDC issues:
Copy link
Contributor

Choose a reason for hiding this comment

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

This was necessary to fix tests failing in master

@greglittlefield-wf greglittlefield-wf marked this pull request as ready for review January 6, 2021 16:12
@greglittlefield-wf greglittlefield-wf changed the title Preparations for React 17 compatibility Preparations for React 17 compatibility, fix master build Jan 6, 2021
@greglittlefield-wf
Copy link
Contributor

This is ready for review @sydneyjodon-wk @aaronlademann-wf (for some reason I could only request review from Sydney 🤷)

FYI @evanweible-wf (fixes master build)

Copy link
Contributor

@aaronlademann-wf aaronlademann-wf left a comment

Choose a reason for hiding this comment

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

+1

@aaronlademann-wf
Copy link
Contributor

QA +1

  • Passing Dart 2.7.0 CI
  • Passing Dart 2.7.0 CI in testing PR
  • No useEffect cleanup / event listener delegation / stopPropagation() / scroll event bubbling usage concerns found.

@Workiva/release-management-p

Copy link
Contributor

@rmconsole-wf rmconsole-wf left a comment

Choose a reason for hiding this comment

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

+1 from RM

@rmconsole6-wk rmconsole6-wk merged commit 6031e6c into master Jan 6, 2021
@rmconsole6-wk rmconsole6-wk deleted the campaign/client_platform/react_17_rollout branch January 6, 2021 18:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants