-
Notifications
You must be signed in to change notification settings - Fork 465
CI test for dependencies to have the same version #1556
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am SO HAPPY you're doing this. The last two website crashes were caused by this. Specifically, they were cause by multiple versions of react-dom
being present.
To illustrate:
├─ [email protected]
├─ [email protected]
│ └─ [email protected]
├─ [email protected]
│ └─ [email protected]
This is caused by react-highlight
in this case, but in the past it has been caused by some of our packages like react-shared
. I just merged https://github.com/0xProject/react-highlight/pull/3 which does this for react-highlight
. If you could change the version of react-highlight
in react-shared
to 0xproject/react-highlight#fix/peer-dependencies
that would be great. I think all React packages that are meant to imported into other React projects should be using peerDependencies
to specify they depend on React. Let the parent project decide the version of react-dom
.
@fragosti thanks for checking! Is there a reason you specifically want to point to the branch Peer deps are new to me, just wanna check I understand before I do it 😌 |
@xianny He want's to point to a specific branch, not to the |
yep, gotcha! I was wondering why we don't use master - do we keep a different state indefinitely on the branches on that repo? Also, looks like that branch got deleted. I'm pointing to master to fix CI for now. I think this is OK but @fragosti please let me know if you need us to do something different. |
works locally but fails on CI 🤔 |
From the build it looks like the types for React might be the wrong version? Not sure. |
Yeah :/ React versions in instant. I reverted versions to what they were before this PR, and it still fails. The last time it worked was with |
* development: update error msg for dependency version check CI test for dependencies to have the same version (#1556) Add missing comma Decode NULL as false Update blog link to not be /latest, and use constant in mobile nav
Description
Add failing CI if there are dependencies with multiple versions.
Can specify dependencies or packages to ignore in /package.json
Testing instructions
Checkout branch, run
yarn remove_node_modules && yarn install
. Then try to build and test everything.Types of changes
Checklist:
[WIP]
if necessary.