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 17 upgrade #2962

Merged
merged 21 commits into from
Jan 29, 2021
Merged

React 17 upgrade #2962

merged 21 commits into from
Jan 29, 2021

Conversation

revanth0212
Copy link
Contributor

@revanth0212 revanth0212 commented Jan 21, 2021

Description

Upgraded React to 17.0.1
Upgraded informed to 3.27.0

Note: Tagging this as a minor change because it is not a breaking change.

Related Issue

Closes PWA-1040

Verification Stakeholders

@dpatil-magento

Verification Steps

  1. Check that the app loads fine.

Screenshots / Screen Captures (if appropriate)

Checklist

  • I have added tests to cover my changes, if necessary.
  • I have added translations for new strings, if necessary.
  • I have updated the documentation accordingly, if necessary.

@PWAStudioBot
Copy link
Contributor

PWAStudioBot commented Jan 21, 2021

Messages
📖

Associated JIRA tickets: PWA-1040.

📖 DangerCI Failures related to missing labels/description/linked issues/etc will persist until the next push or next nightly build run (assuming they are fixed).
📖

Access a deployed version of this PR here. Make sure to wait for the "pwa-pull-request-deploy" job to complete.

Generated by 🚫 dangerJS against f26c8c8

@revanth0212 revanth0212 changed the title Revanth/react 17 upgrade React 17 upgrade Jan 22, 2021
@revanth0212
Copy link
Contributor Author

Ignore the tests for now. The new version of informed is adding ids to the input fields and they change every time the component renders. Ill find a way to handle them. Also, not sure why app.spec.snap is 40k lines of change 🙄 . Ill check on that as well.

@revanth0212 revanth0212 marked this pull request as ready for review January 23, 2021 00:53
@revanth0212
Copy link
Contributor Author

Ignore the tests for now. The new version of informed is adding ids to the input fields and they change every time the component renders. Ill find a way to handle them. Also, not sure why app.spec.snap is 40k lines of change 🙄 . Ill check on that as well.

There is a huge change in the app.spec.js.snap because of the new fiber nodes that are inserted into the React DOM.

I'll see if I can mock them but since this is an internal object inside of the react's huge code base, no guarantees 🤞 .

image

@jimbo
Copy link
Contributor

jimbo commented Jan 27, 2021

I suggest using ShallowRenderer if the snapshots are becoming unwieldy with React 17. No need to assert the rendered output of child components; their own unit tests can do that.

Also, I believe this must be a major release for us, not a minor. There's no way someone could reasonably pull these changes without disruption.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
…on/creditCard.js

Co-authored-by: Andy Terranova <13182778+supernova-at@users.noreply.github.com>
revanth0212 and others added 3 commits January 28, 2021 14:27

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
supernova-at
supernova-at previously approved these changes Jan 28, 2021
@supernova-at
Copy link
Contributor

This is 👍 but Danger is failing - looks like you just need to run prettier @revanth0212

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
@dpatil-magento
Copy link
Contributor

QA Approved.

@dpatil-magento dpatil-magento merged commit ddc704d into develop Jan 29, 2021
@dpatil-magento dpatil-magento deleted the revanth/react_17_upgrade branch January 29, 2021 21:12
@dpatil-magento dpatil-magento added version: Major This changeset includes incompatible API changes and its release necessitates a Major version bump. and removed version: Minor This changeset includes functionality added in a backwards compatible manner. labels Mar 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pkg:extensions pkg:pagebuilder pkg:peregrine pkg:pwa-buildpack pkg:venia-concept pkg:venia-ui Progress: done version: Major This changeset includes incompatible API changes and its release necessitates a Major version bump.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants