search

brainhubeu / react-carousel

A pure extendable React carousel, powered by Brainhub (craftsmen who ❤️ JS)
https://brainhub.eu/
MIT License
1.07k stars 164 forks source link

Fix production builds using webpack #646

Closed geo-mathijs closed 3 years ago

geo-mathijs commented 4 years ago

It appears that react and react-dom were specified in both the peerDependencies and normal dependencies. There are some issues when packaging react apps with webpack when multiple copies of React are present in the same (packaged) app (see also: https://reactjs.org/warnings/invalid-hook-call-warning.html).

The webpack config is still not ideal, as it does not set the production environment but just happens to default to it. I have also found some weird behavior with the manual set on L51 and Recoil, but this is not the root cause and only causes a "degrade" to development mode in Recoil. This may be better suited for a different issue however.

For now the builds seem to work locally, let me know if more information is needed.

cor14095 commented 3 years ago

Is anyone working on this PR?

Lukasz-pluszczewski commented 3 years ago

Hi, sorry for not answering for so long.

Seems like E2E tests failed (the CI deploy_test-env and test steps are just incorrectly configured, we'll fix that in the future, but E2E tests actually fail), nothing seems to be actually broken. I'm also trying to figure out what's going on.

Also, please update your commit messages in accordance with https://github.com/semantic-release/semantic-release. I think fix: is appropriate here.

geo-mathijs commented 3 years ago

Hi! I have amended the commit messages to include fix: and merged the latest commits. All the automated tests are having trouble again however, but I'm not exactly sure why either.

Lukasz-pluszczewski commented 3 years ago

Hi, sorry for the delay, we have a really busy time. We'll look into the issue with E2E tests as soon as possible. Sorry once again.