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 react-markdown in production #727

Merged
merged 1 commit into from
Apr 16, 2021
Merged

Fix react-markdown in production #727

merged 1 commit into from
Apr 16, 2021

Conversation

cquiroz
Copy link
Contributor

@cquiroz cquiroz commented Apr 16, 2021

react-markdown is being used for the help system but it fails in production.
After chasing down this issue, my conclusion is that vite/rollup has some troubles with modules using commonjs style rather than es style

vitejs/vite#2862

This is still odd because it works properly if I use it on the plain js side, thus there is something odd on the way scala.js interacts with commonjs imports

The solution I found is kind of convoluted but seems to work:

Remove the react-markdown dependency and internalize the .js files from react-markdown but convert them to es via cjstoesem

This feels very hacky but the only alternative I can think of is to reimplement in scala.
I'd like to at least test this in prod.

Signed-off-by: Carlos Quiroz <cquiroz@gemini.edu>
@cquiroz cquiroz force-pushed the react-markdown-ugly-fix branch from 0a2f7f2 to d79add5 Compare April 16, 2021 18:40
Copy link
Contributor

@rpiaggio rpiaggio left a comment

Choose a reason for hiding this comment

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

👍 hope this fixes the issue in prod

Copy link
Contributor

@toddburnside toddburnside left a comment

Choose a reason for hiding this comment

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

🤞 👍

@cquiroz cquiroz merged commit 68d8645 into master Apr 16, 2021
@cquiroz cquiroz deleted the react-markdown-ugly-fix branch April 16, 2021 19:00
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