react-component / animate

anim react element easily
http://react-component.github.io/animate/
MIT License
679 stars 108 forks source link

Specify peerDependencies (for compatibility with Yarn 2.0) #99

Closed rlue closed 4 years ago

rlue commented 4 years ago

The Problem

rc-animate depends on rc-util, which peer-depends on react and react-dom. The latest version of Yarn (2.0 / berry) does not handle this peer dependency inheritance automatically, instead requiring peer dependencies to be explicitly specified at every level of the dependency tree:

# not ok
project
├── react
├── react-dom
└── rc-animate
    └── rc-util
        ├─peer─ react
        └─peer─ react-dom

# ok
project
├── react
├── react-dom
└── rc-animate
    ├─peer─ react
    ├─peer─ react-dom
    └── rc-util
        ├─peer─ react
        └─peer─ react-dom

As currently configured, rc-animate produces the following console warnings when being added to a yarn 2.0 project:

$ mkdir bugreport
$ cd bugreport
$ yarn set version berry
$ yarn add react
$ yarn add react-dom
$ yarn add rc-animate
➤ YN0000: ┌ Resolution step
➤ YN0002: │ rc-animate@npm:2.10.3 doesn't provide react@^15.0.0 || ^16.0.0 requested by rc-util@npm:4.20.1
➤ YN0002: │ rc-animate@npm:2.10.3 doesn't provide react-dom@^15.0.0 || ^16.0.0 requested by rc-util@npm:4.20.1
➤ YN0000: └ Completed in 6.13s
➤ YN0000: ┌ Fetch step
➤ YN0000: └ Completed in 0.34s
➤ YN0000: ┌ Link step
➤ YN0000: └ Completed in 1.82s
➤ YN0000: Done with warnings in 8.31s

This requirement is documented here and explained in detail here.

The Solution

package.json needs a peerDependencies property that matches the contents of the one in rc-util.

Note

This same issue applies to all react-component repos that depend on rc-util or any other package that peer-depends on react and react-dom. Presumably, this is every project until the react-component umbrella. I'm too lazy to create separate issues on each one; is there a better way to report this issue across the entire project?

brandonbloom commented 4 years ago

I can't find any stated rationale for the associated "remove peerDependencies" commits across several related repositories. I think the correct fix is to add peer dependencies to this and related packages, instead of removing them from this package's dependencies.

Now, instead of warnings during resolution, you get errors at build or runtime:

ModuleNotFoundError: Module not found: Error: rc-animate tried to access react-dom, but it isn't declared in its dependencies; this makes the require call ambiguous and unsound.
Required package: react-dom (via "react-dom")
Required by: rc-animate@npm:3.1.1 (via /Users/brandonbloom/Projects/deref/.yarn/cache/rc-animate-npm-3.1.1-ebc3b6785f-bc3baf9704.zip/node_modules/rc-animate/es/AnimateChild.js)