search

LavaMoat / LavaDome

Secure DOM trees isolation and encapsulation leveraging ShadowDOM
https://lavamoat.github.io/LavaDome/packages/core/demo/
MIT License
21 stars 4 forks source link

feat(react): support react v17, v18 #43

Closed legobeat closed 2 months ago

legobeat commented 4 months ago

Allows using the package with react versions 17 and 18.

legobeat commented 4 months ago

@LavaMoat/devs Ping. Reviews not enabled for me on this repo :sweat_smile:

legobeat commented 3 months ago

The branch should work, the only user-facing change is widening of peerDependencies

legobeat commented 2 months ago

@weizman what is the remaining concern blocking the merge of this?

weizman commented 2 months ago

This is https://github.com/LavaMoat/LavaDome/pull/43#pullrequestreview-2092364367

legobeat commented 2 months ago

This is #43 (review)

Yes - what sort of validation would you like to see?

weizman commented 2 months ago

When I do:

mkdir delete1
cd delete1
git clone https://github.com/LavaMoat/LavaDome/
cd lavadome
git remote add upstream https://github.com/legobeat/LavaDome/
git fetch --all
git checkout react-support-18
nvm use 20
npm install

I get:

➜  lavadome git:(react-support-18) npm install
npm ERR! code ERESOLVE
npm ERR! ERESOLVE unable to resolve dependency tree
npm ERR!
npm ERR! While resolving: @lavamoat/lavadome-react@0.0.17
npm ERR! Found: react@18.3.1
npm ERR! node_modules/react
npm ERR!   dev react@"^18.3.1" from @lavamoat/lavadome-react@0.0.17
npm ERR!   packages/react
npm ERR!     @lavamoat/lavadome-react@0.0.17
npm ERR!     node_modules/@lavamoat/lavadome-react
npm ERR!       workspace packages/react from the root project
npm ERR!
npm ERR! Could not resolve dependency:
npm ERR! peer react@"<18.0.0" from @testing-library/react@12.1.5
npm ERR! node_modules/@testing-library/react
npm ERR!   dev @testing-library/react@"^12.1.2" from @lavamoat/lavadome-react@0.0.17
npm ERR!   packages/react
npm ERR!     @lavamoat/lavadome-react@0.0.17
npm ERR!     node_modules/@lavamoat/lavadome-react
npm ERR!       workspace packages/react from the root project
npm ERR!
npm ERR! Fix the upstream dependency conflict, or retry
npm ERR! this command with --force or --legacy-peer-deps
npm ERR! to accept an incorrect (and potentially broken) dependency resolution.
npm ERR!
npm ERR!

Am I doing something wrong?

legobeat commented 2 months ago

@testing-library/react@12.1.5

I do not get that, but I guess that's because different versions on npm handle peerDependency violations differently (so declaring a specific packageManager should improve these kinds of confusions, rf #44 )

In this case, it's because the devDependency @testing-library/react@12.1.5 is not indicating support for the version of react being tested for (but obviously it works ;))

weizman commented 2 months ago

I just merged #51 - correct me if I'm wrong, but #51 included both commits coming from here(#43), so this PR can be closed @legobeat ?

legobeat commented 2 months ago

I just merged #51 - correct me if I'm wrong, but #51 included both commits coming from here(#43), so this PR can be closed @legobeat ?

Yes, in fact it auto-closes by syncing the branch.

Though it'd make for more obvious history and PR states if merged in order (which would have made this one show as "merged" rather than "closed")