Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix version mismatch error #1180

Closed
wants to merge 1 commit into from
Closed

Conversation

srosset81
Copy link
Contributor

@srosset81 srosset81 commented Oct 16, 2023

Since devDependencies were added to the @semapps/date-components and @semapps/markdown-components, using yarn link with these packages produce many problems:

  • The build is 2 or 3 times slower
  • This warning is being displayed: You are loading @emotion/react when it is already loaded. Running multiple instances may cause problems. This can happen if multiple versions are used, or if multiple builds of the same version are used.
  • As soon as you use a React hook, you have this fatal error https://react.dev/warnings/invalid-hook-call-warning which warns about a version mismatch.

@Laurin-W could reproduce this as well.

After more than 2 hours of search, here are the most significant issues:

The last issue indicate that the problem appears when React (and probably MUI) are in dev dependencies.

So I removed all the devDependencies from @semapps/date-components and @semapps/markdown-components (except parcel) in this PR and it's now working fine.

@mguihal @Laurin-W Can we do without these devDependencies ? What problems does it cause, except maybe eslint warnings ?

Two other solutions:

  • Add a reverse link to the react (and other packages), as noted in the two issues above. But this is very complicated to setup, because it must be changed whenever we work on another app. Plus it doesn't work if we work simultaneously on two apps (this happened for me).

  • Revert to the pre-Parcel config (as in v0.5). But there was many problems. And I don't really know why it worked, the risk is that this problems appears again.

@srosset81 srosset81 marked this pull request as ready for review October 16, 2023 16:37
@srosset81
Copy link
Contributor Author

For information, here are some alternatives to Parcel, but they seem to have the same issue:

Copy link
Contributor

@mguihal mguihal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed I reproduce this issue too.

Your updates are breaking the typescript build on dates-components and markdown-components (if you build after erasing/reinstalling your node_modules in those packages and in the frontend folder), so we can't do that.

These dev dependencies are required by Typescript for typing correctly the packages, and I can find an easy solution to ignore that. I tested parcel alias but it doesn't work, and I tested to install these dev dependencies in the frontend folder instead of each package folder, but it doesn't seem to solve the problem :/

It seems it's an issue with how yarn link is working and it can be mitigated by using yalc instead of yarn link :

yarn global add yalc

# In semapps/markdown-components folder (on "next" branch for example)
yalc publish

# In app using semapps
yalc link @semapps/markdown-components

# In semapps/markdown-components folder whenever you make changes
yarn build && yalc publish --push

Can you confirm this solution works for you too? Can it be an acceptable solution?

@srosset81
Copy link
Contributor Author

srosset81 commented Oct 17, 2023

Thanks for the investigation and the proposal @mguihal
So we would need to publish to yalc whenever we switch of SemApps branch? Maybe this could be automated with some kind of git hooks ?

@Laurin-W
Copy link
Contributor

@mguihal your solution works for me.
I think the big downside here is that it requires to run yalc publish after every change made in semapps/frontend. When developing in semapps and testing in activitypods or archipelago this might be pretty annoying. Running yalc publish with git hooks is fine but I suppose, we'd have to add a file watcher to get it smooth?

@srosset81 srosset81 mentioned this pull request Oct 17, 2023
3 tasks
@srosset81
Copy link
Contributor Author

Closing this PR in favor of #1181 See you there for the following discussions :-)

@srosset81 srosset81 closed this Oct 17, 2023
@srosset81 srosset81 deleted the fix-version-mismatch-error branch October 17, 2023 16:26
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.

3 participants