search

carbon-design-system / carbon

A design system built by IBM
https://www.carbondesignsystem.com
Apache License 2.0
7.75k stars 1.8k forks source link

Upgrade class components to functional components #9712

Closed joshblack closed 2 years ago

joshblack commented 3 years ago

Why are we doing this?

According to the React team, switching a component from a class component to a functional component is technically a breaking change. This is because class components accept a ref that points to the class instance by default. There is no way to opt-out of this behavior. As a result, if we switched a component to a functional component it would no longer have a ref, and existing code that uses a ref would break. Similarly, using React.forwardRef would produce a component where the ref would no longer point to a class instance and code could similarly break.

We have moved some components over to the new format despite these situations. The rationale at the time was that we weren't intending people to use these parts of a component (specifically class instance methods) and if teams were trying to use the ref to find a node in the document their code would work if we used React.forwardRef.

With v11, it seemed like a great opportunity to convert our remaining class components to the new functional component style with hooks.

Effort size legend ❇️ - sm ✴️ - md 🆘 - lg

Tasks

We would like to convert the following components to functional components that use hooks where appropriate:

joshblack commented 3 years ago

Recipes

// TEMPLATE
function ComponentName(props, ref) {}

const ComponentName = React.forwardRef(
  function ComponentName(props, ref) {}
);

ComponentName.propTypes = {};
  1. Start off with the template of the component
  2. Grab the render() method and paste it into your new component
  3. Grab the propTypes and paste them into your new component
  4. Move over any default props, where appropriate
  5. Initialize state in local variables using React.useState from the constructor
  6. Move over any event handlers into the body of the component
  7. Fix everything that breaks (change this.* to local variables)

Special cases

Situations where we would not want to migrate

How to use in monorepo

import * as FeatureFlags from '@carbon/feature-flags';
import { OverflowMenu as OverflowMenuNext } from './next/OverflowMenu';
import OverflowMenuClassic from './OverflowMenu';

export const OverflowMenu = FeatureFlags.enabled('enable-v11-release')
  ? OverflowMenuNext
  : OverflowMenuClassic;
sstrubberg commented 2 years ago

example for how to handle getDerivedStateFromProps


function TestComponent({ example }) {
  const [value, setValue] = useState(example);

  useEffect(() => {
    setValue(example);
  }, [example]);
}

becomes


function TestComponent({ example }) {
  const [value, setValue] = useState(example);
  const [prevExample, setPrevExample] = useState(example);

  if (example !== prevExample) {
    setValue(example);
    setPrevExample(example);
  }
}
tay1orjones commented 2 years ago

Another thing to note on default vs named exports - the src/index.js is typically expecting a default export. So if you're going to do a conditional export, it might be easiest to ensure it's default exports all the way through the export chain.

For instance:

// MyFunComponent/next/MyFunComponent.js

export default function MyFunComponent({...props}) {
return (
    <div>{...props}</div>
  );
}

// MyFunComponent/index.js
import { default as MyFunComponentNext } from './next/MyFunComponent';
import { default as MyFunComponentClassic } from './MyFunComponent';

export default MyFunComponent = FeatureFlags.enabled('enable-v11-release') ? MyFunComponentNext : MyFunComponentClassic;
jnm2377 commented 2 years ago

We also need to add a new test file in the /next folder for each component and update the test to pull in the v11 component.

joshblack commented 2 years ago

Checklist

Questions

motou commented 2 years ago

I can take over the task for the component:

jnm2377 commented 2 years ago

Awesome! Thanks @motou we appreciate the contribution! I'll assign it to you. If you have any questions about the implementation, feel free to look at the other PRs we've opened so far, or ask questions here. 😄

motou commented 2 years ago

@jnm2377 would you please approve the running workflows for https://github.com/carbon-design-system/carbon/pull/9955? Thanks!

motou commented 2 years ago

@joshblack

Checklist

  • [ ] Add an overview of these changes to the v11-carbon-components-react.md file

the right file path is docs/migration/11.x-carbon-components-react.md

motou commented 2 years ago

Moreover, I can work on the following components:

tay1orjones commented 2 years ago

Priority of these can be divided into three sections:

1. Components that are likely to receive enhancement work post v11

2. Components that are commonly used

3. Everything else - Wrap these classes with a functional export

tay1orjones commented 2 years ago

Looks like this can be marked as complete based on status of https://github.com/carbon-design-system/carbon/issues/10281 👍