Skip to content

Conversation

@austingreendev
Copy link
Contributor

@austingreendev austingreendev commented May 20, 2019

  • BREAKING CHANGE

Description

Here is the final React 16.8 and styled-components v4.2 upgrade PR! This will eventually be paired with our upcoming versioning changes.

This is a BREAKING CHANGE which requires peerDependency changes, styled-components deprecation removals, and innerRef removal due to the React.forwardRef API.

Detail

This PR is split into several commits to allow for easy review. All changes have gone through an initial review when being merged into this branch, but you can review each package upgrade by its commit.

Checklist

  • 👌 design updates are Garden Designer approved (add the
    designer as a reviewer)
  • 💅 view component styling is based on a Garden CSS
    component
  • 🌐 Styleguidist demo is up-to-date (yarn start)
  • ⬅️ renders as expected with reversed (RTL) direction
  • ♿ analyzed via axe and evaluated using VoiceOver
  • 💂‍♂️ includes new unit tests
  • 📝 tested in Chrome, Firefox, Safari, Edge, and IE11

@austingreendev austingreendev requested a review from a team May 20, 2019 18:23
@austingreendev austingreendev marked this pull request as ready for review May 20, 2019 18:23
@coveralls
Copy link

coveralls commented May 20, 2019

Coverage Status

Coverage decreased (-0.03%) to 94.752% when pulling bd94f84 on agreen/react-16-upgrade into 930d97a on master.

Copy link
Contributor

@ryanseddon ryanseddon left a comment

Choose a reason for hiding this comment

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

Nice work!

I think you should do the testing deletion as a separate PR so as not to bury it here, we should also publish a deprecation notice to the package so anyone installing can see that.

Copy link
Member

@jzempel jzempel left a comment

Choose a reason for hiding this comment

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

brilliant work here, @Austin94. I just have a few clean-up items:

  • Remove the invalid "Installation" and "Requirements" for:
    • dropdowns/README.md
    • forms/README.md
  • Replace Fragment with <> usage on packages/buttons/README.md (also </Fragement> is misspelled)
  • Fix “install peer dependencies” on root README.md
  • It probably makes sense to drop the tag prop from typography components, now that we have as via styled-components.
  • Please consider if it makes sense to complete the "deprecate packages replaced by react-forms" task before this PR is merged.

Thanks!

@austingreendev
Copy link
Contributor Author

Fix “install peer dependencies” on root README.md

This doesn't include a version anywhere. What needs fixing?

It probably makes sense to drop the tag prop from typography components, now that we have as via styled-components.

I'm mixed about this. The as prop is specific to our usage of styled-components. I don't see a benefit with us moving this over at this time since it's one of the few components where we expect users to heavily customize the base tag.

Please consider if it makes sense to complete the "deprecate packages replaced by react-forms" task before this PR is merged.

Let's do that as a follow on. Would be a great excuse to give the new versioning, release, and changelog management flow a test.

@jzempel
Copy link
Member

jzempel commented May 23, 2019

@Austin94 the list is missing peer dependencies required for react-buttons

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

5 participants