Skip to content

Conversation

@Almaju
Copy link
Contributor

@Almaju Almaju commented Sep 22, 2025

Because this library is consumed by other apps, we want react to be listed as a peer dependency to let the main app manage the version. We use >= 19.0.0 not to be restrictive with the minor and patch versions.
We also add it to the devDependencies to be able to use React in tests.
I took inspiration of other open source JS libraries (example: https://github.com/mobxjs/mobx-react-lite/blob/master/package.json or https://github.com/Effect-TS/effect/blob/main/packages/rpc/package.json).

@Almaju Almaju requested a review from a team as a code owner September 22, 2025 14:43
@justindarc
Copy link
Contributor

Can you also update the React example in /examples to pull a different React version to demonstrate that this is now working as we intend. I think doing this will also make it can act as a basic smoketest with our integration testing if the React example fails to build.

@Almaju Almaju force-pushed the MAJC-174-new-hire-make-sure-that-our-react-dependency-requirement-doesnt-conflict-with-the-consumers-react branch from 4dc0682 to 8ba61fa Compare September 22, 2025 20:38
@Almaju
Copy link
Contributor Author

Almaju commented Sep 22, 2025

Can you also update the React example in /examples to pull a different React version to demonstrate that this is now working as we intend. I think doing this will also make it can act as a basic smoketest with our integration testing if the React example fails to build.

@justindarc I tested different React versions in examples/react/package.json and then npm i with both >= 19.0.0 and previous versions. I correctly have only one react dependency in node_modules at the end but this might need to be tested against the published npm package (instead of a local path in tsconfig).

In case of a version mismatch, it gets overridden by npm with a warning:

npm warn ERESOLVE overriding peer dependency
npm warn While resolving: @mozilla/majc-react-example@0.1.0
npm warn Found: react@19.0.0
npm warn node_modules/react
npm warn   react@"18.0.0" from the root project
npm warn   3 more (next, react-dom, styled-jsx)

Not sure what I should commit specifically in the React example to demonstrate that it works since the peerDependencies specifies >= 19.0.0 which would work with any version currently compatible with Nextjs 15.

@Almaju Almaju force-pushed the MAJC-174-new-hire-make-sure-that-our-react-dependency-requirement-doesnt-conflict-with-the-consumers-react branch from 8ba61fa to 825afc1 Compare September 22, 2025 20:51
@justindarc
Copy link
Contributor

Good call. Let's go with this for now, get it merged, then we can publish and do a follow-up PR if possible to demonstrate what we were trying to do. Thanks for looking into it!

…ependency-requirement-doesnt-conflict-with-the-consumers-react
@Almaju Almaju merged commit 73e9844 into mozilla-services:main Sep 23, 2025
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants