reactjs / react-transition-group

An easy way to perform animations when a React component enters or leaves the DOM
https://reactcommunity.org/react-transition-group/
Other
10.1k stars 652 forks source link

react-transition-group demo using TypeScript no longer works when upgrading to @types/react & @types/react-dom v18 #813

Closed kimbaudi closed 2 years ago

kimbaudi commented 2 years ago

What is the current behavior?

I am using react v18, react-dom v18, react-transition-group v4.4.2, @types/react v18, @types/react-doom v18, and @types/react-transition-group v4.4.4

Before upgrading @types/react from v17.0.43 to v.18 and @types/react-dom from v17.0.14 to v.18, my react-transition-demo using TypeScript had no lint errors and worked fine.

After upgrading @types/react & @types/react-dom to v18, I get 2 lint errors:

'Transition' cannot be used as a JSX component.
  Its instance type 'Transition<HTMLElement>' is not a valid JSX element.
    The types returned by 'render()' are incompatible between these types.
      Type 'import("mytsreactapp/node_modules/@types/react-dom/node_modules/@types/react/index").ReactNode' is not assignable to type 'React.ReactNode'.
        Type '{}' is not assignable to type 'ReactNode'.ts(2786)
Type 'TransitionChildren' is not assignable to type 'ReactNode'.
  Type '{}' is not assignable to type 'ReactNode'.ts(2322)
index.d.ts(1360, 9): The expected type comes from property 'children' which is declared here on type 'ClassAttributes<HTMLDivElement> & HTMLAttributes<HTMLDivElement> & { css?: Interpolation<Theme>; }'

I believe the issue is that @types/react-transition-group no longer works with @types/react v18 and @types/react-dom v18.

What is the expected behavior?

I expected no TypeScript lint errors after upgrading @types/react & @types/react-dom to v18.

Could you provide a CodeSandbox demo reproducing the bug?

I'll won't be able to provide a CodeSandbox demo reproducing the bug until later this evening. Until then, here is my SimpleFadeDemo.tsx react component that has TS lint errors after upgrading @types/react from v17.0.43 to v.18 and @types/react-dom from v17.0.14 to v.18:

SimpleFadeDemo.tsx:

import { useRef, useState } from 'react';
import {
  TransitionProps,
  TransitionStatus,
} from 'react-transition-group/Transition';
import { Transition } from 'react-transition-group';
import CSS from 'csstype';
import { EmotionJSX } from '@emotion/react/types/jsx-namespace';

const duration = 300;

const defaultStyle = {
  transition: `opacity ${duration}ms ease-in-out`,
  opacity: 0,
};

const transitionStyles: Partial<Record<TransitionStatus, CSS.Properties>> = {
  entering: { opacity: 1 },
  entered: { opacity: 1 },
  exiting: { opacity: 0 },
  exited: { opacity: 0 },
};

const Fade = function Fade({
  in: inProp,
  children,
}: TransitionProps<HTMLElement>) {
  const nodeRef = useRef<HTMLElement>(null);

  return (
    <Transition nodeRef={nodeRef} in={inProp} timeout={duration}>
      {(state) => (
        <div
          style={{
            ...defaultStyle,
            ...transitionStyles[state],
          }}
        >
          {children}
        </div>
      )}
    </Transition>
  );
};

const SimpleFadeDemo = function SimpleFadeDemo(): EmotionJSX.Element {
  const [inProp, setInProp] = useState(false);
  const toggle = () => setInProp((state) => !state);

  return (
    <div>
      <button type="button" onClick={toggle}>
        Toggle Fade
      </button>
      <Fade
        in={inProp}
        // timeout={500}
        timeout={{
          appear: 500,
          enter: 300,
          exit: 500,
        }}
      >
        <ul>
          <li>List 1</li>
          <li>List 2</li>
          <li>List 3</li>
          <li>List 4</li>
          <li>List 5</li>
        </ul>
      </Fade>
    </div>
  );
};

export default SimpleFadeDemo;

image

seanblonien commented 2 years ago

v17 also doesn't work for that matter. I get the exact same error

mrvisser commented 2 years ago

Bear with me as I handwave my shallow knowledge of yarn dependency resolution, transitive dependencies and hoisting right now...

The react-transition-group's yarn.lock file right now specifies @types/react@* => 16.9.34. Which is maybe a small problem, but when pulling in react-transition-group as a transitive dependency shouldn't be affecting you. I can't seem to find any module that RTG depends on that is hard-coding a @types/react dependency, afaict it's all @types/react@*, which should be correct.

But, it's possible that the same set of dependency upgrades that RTG is doing to get up to react@18 and leaving behind a @types/react@16.9.34 might also be leaving some cruft in your lock file saying "RTG wants @types/react@16.9.34 specifically".

Might want to check your lock file and see if you have anything that says:

"react-transition-group depends on @types/react@*"

and then:

"@types/react@* should be version <the unhoisted version assigned in node_modules/react-transition-group/node_modules/@types/react/package.json" (maybe the most relevant version of @types/react when you initially installed it)

Meanwhile, your package.json file could be saying "@types/react": "^18.0.0", with a lock file entry that says "@types/react@^18.0.0 should be version 18.0.0".

Then you may end up with a package manager like yarn putting an "unhoisted" older version into the react-transition-group transitive node_modules dir. If that's the problem, I'm not sure what the proper solution is, though.

mrvisser commented 2 years ago

Maybe some form of this: https://github.com/yarnpkg/yarn/issues/1785

Which seems to have this as a workaround: https://classic.yarnpkg.com/lang/en/docs/selective-version-resolutions/

... if you're using yarn.

kimbaudi commented 2 years ago

Also possibly related: https://github.com/DefinitelyTyped/DefinitelyTyped/issues/59765 and https://github.com/reduxjs/react-redux/issues/1886.

The 'Transition' cannot be used as a JSX component error happens because upgrading to @types/react v18 and @types/react-dom v18 causes 2 versions of @types/react to be installed (pre v18 and v18).

I'm using yarn so I am resolving this issue by using overrides in my package.json:

  "overrides": {
    "@types/react": "17.0.43",
    "@types/react-dom": "17.0.14"
  },

Once @types/react-transition-group updates its dependency of @types/react and @types/react-dom to support v18, I think I can get rid of the overrides in my package.json and I will no longer receive this error.

kimbaudi commented 2 years ago

I no longer get the error: The 'Transition' cannot be used as a JSX component. All I did was delete node_modules folder and yarn.lock file and reinstall all my dependencies with the latest versions. When I did this yesterday, I still got the error, but not after I tried today.

Also, I am no longer resolving this issue by using overrides or resolutions in my package.json.

kimbaudi commented 2 years ago

The error The expected type comes from property 'children' which is declared here on type 'ClassAttributes<HTMLDivElement> & HTMLAttributes<HTMLDivElement> & { css?: Interpolation<Theme>; }' seems to be related to a breaking change - removal of implicit children (see https://github.com/DefinitelyTyped/DefinitelyTyped/pull/56210).

BTW, I'm also getting this children props TS lint error from react-helmet-async npm package and it seems there is a PR to fix the issue. (see https://github.com/staylor/react-helmet-async/issues/163, https://github.com/staylor/react-helmet-async/pull/164, and https://github.com/staylor/react-helmet-async/issues/166).

However, I was able to fix the children props TS lint error in my react-transition-group demo using Typescript by updating my Fade component props to include children prop using React.PropsWithChildren generic type.

Now, I can now use the latest react v18, react-dom v18, @types/react v18, and @types/react-dom v18 and no longer get the TS lint error on my react-transition-group demo using TypeScript.

Here is the updated Fade component using react-transition-group and @types/react-transition-group:

Fade.tsx:

import { PropsWithChildren, useRef } from 'react';
import {
  TransitionProps,
  TransitionStatus,
} from 'react-transition-group/Transition';
import { Transition } from 'react-transition-group';
import CSS from 'csstype';

const duration = 300;

const defaultStyle = {
  transition: `opacity ${duration}ms ease-in-out`,
  opacity: 0,
};

const transitionStyles: Partial<Record<TransitionStatus, CSS.Properties>> = {
  entering: { opacity: 1 },
  entered: { opacity: 1 },
  exiting: { opacity: 0 },
  exited: { opacity: 0 },
};

function Fade({
  in: inProp,
  children,
}: PropsWithChildren<TransitionProps<HTMLDivElement>>) {
  const nodeRef = useRef<HTMLDivElement>(null);

  return (
    <Transition nodeRef={nodeRef} in={inProp} timeout={duration}>
      {(state) => (
        <div
          style={{
            ...defaultStyle,
            ...transitionStyles[state],
          }}
        >
          {children}
        </div>
      )}
    </Transition>
  );
}

export default Fade;
kimbaudi commented 2 years ago

Closing this issue since I am no longer getting TS lint errors on my react-transition-group demo using Typescript, Also, I probably should create a ticket for this in https://github.com/DefinitelyTyped/DefinitelyTyped/issues next time.

mrvisser commented 2 years ago

For those who are running projects where it's not appropriate to delete your yarn.lock file and let yarn rebuild it, I did the following to fix the issue:

Override @types/react to 18.0.0 your package.json

  "resolutions": {
    "**/@types/react": "^18.0.0"
  },

.. replace the ** and do a bit of trial and error if you want to fix the resolution to the react-transition-group packages.

Run yarn install

For me, it made this change, essentially remapping @types/react@* from 17 up to ^18.0.1:

-"@types/react@*":
-  version "17.0.19"
-  resolved "https://registry.yarnpkg.com/@types/react/-/react-17.0.19.tgz#8f2a85e8180a43b57966b237d26a29481dacc991"
-  integrity sha512-sX1HisdB1/ZESixMTGnMxH9TDe8Sk709734fEQZzCV/4lSu9kJCPbo2PbTRoZM+53Pp0P10hYVyReUueGwUi4A==
-  dependencies:
-    "@types/prop-types" "*"
-    "@types/scheduler" "*"
-    csstype "^3.0.2"
-
-"@types/react@^18.0.0":
-  version "18.0.0"
-  resolved "https://registry.yarnpkg.com/@types/react/-/react-18.0.0.tgz#4be8aa3a2d04afc3ac2cc1ca43d39b0bd412890c"
-  integrity sha512-7+K7zEQYu7NzOwQGLR91KwWXXDzmTFODRVizJyIALf6RfLv2GDpqpknX64pvRVILXCpXi7O/pua8NGk44dLvJw==
+"@types/react@*", "@types/react@^18.0.0", "@types/react@^18.0.1":
+  version "18.0.1"
+  resolved "https://registry.yarnpkg.com/@types/react/-/react-18.0.1.tgz#1b2e02fb7613212518733946e49fb963dfc66e19"
+  integrity sha512-VnWlrVgG0dYt+NqlfMI0yUYb8Rdl4XUROyH+c6gq/iFCiZ805Vi//26UW38DHnxQkbDhnrIWTBiy6oKZqL11cw==

Now you can remove the "resolutions" section you just added and run yarn install

Once you've remapped @types/react@*, it will stay there without resolutions until the next time you upgrade and you might have to do it again :) Removing from "resolutions" is optional, but I think safer in case there are proper breaking changes in React that are reflected by the types, you may want the types to help you point them out.