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 "Export SVG" feature in next 14 #4136

Merged
merged 9 commits into from
Dec 14, 2023
Merged

Fix "Export SVG" feature in next 14 #4136

merged 9 commits into from
Dec 14, 2023

Conversation

cmdcolin
Copy link
Collaborator

Next.js version 14 currently throws an error when trying to export svg. The svg rendering uses a function renderToStaticMarkup from "react-dom/server" and next js decided it doesn't like seeing that in client code. Using a recommendation from the react docs goes some way towards fixing https://react.dev/reference/react-dom/server/renderToString#removing-rendertostring-from-the-client-code

xref vercel/next.js#58533 (comment)
fixes GMOD/jbrowse-react-app-nextjs-demo#1

Note: We still use renderToString in the web worker. We do not document web worker usage of our embedded components under nextjs though, but if this ever happened (#3730 ) then it may run into this, and the workaround in this PR would not work because it assumes existence of a document which does not exist in the web worker

Copy link

codecov bot commented Dec 14, 2023

Codecov Report

Attention: 5 lines in your changes are missing coverage. Please review.

Comparison is base (6e061fe) 63.12% compared to head (13f90b0) 63.12%.

Files Patch % Lines
packages/core/util/index.ts 81.81% 2 Missing ⚠️
products/jbrowse-react-app/src/createViewState.ts 0.00% 2 Missing ⚠️
...view/src/BreakpointSplitView/svgcomponents/util.ts 91.66% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #4136   +/-   ##
=======================================
  Coverage   63.12%   63.12%           
=======================================
  Files        1062     1064    +2     
  Lines       31044    31059   +15     
  Branches     7397     7399    +2     
=======================================
+ Hits        19595    19605   +10     
- Misses      11279    11285    +6     
+ Partials      170      169    -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@cmdcolin
Copy link
Collaborator Author

other notes:

  • this further makes @jbrowse/img dependent on jsdom, but it already did use jsdom so not really a change there.
  • reading from innerHTML changes some attributes such as > to > which we then flip back manually. this doesn't seem ideal, but it does match snapshot tests after doing so

@cmdcolin cmdcolin merged commit e6d30a8 into main Dec 14, 2023
10 checks passed
@cmdcolin cmdcolin deleted the next14_exportsvg branch December 14, 2023 17:40
@cmdcolin cmdcolin added the bug Something isn't working label Dec 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unable to use "Export SVG" with Next 14
1 participant