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

DiagrammeRsvg broken with the current development version of DiagrammeR #526

Open
krlmlr opened this issue Dec 15, 2024 · 7 comments
Open

Comments

@krlmlr
Copy link

krlmlr commented Dec 15, 2024

After #525:

dm::dm(a = data.frame(x = 1)) |>
  dm::dm_draw() |>
  DiagrammeRsvg::export_svg()
#> Error: TypeError: Viz is not a function

Created on 2024-12-15 with reprex v2.1.1

How does the conversion to SVG work these days?

Downstream GitHub workflow: https://github.com/cynkra/dm/actions/runs/12267265984/job/34227313989#step:9:305

CC @Yinimi @rich-iannone.

@Yinimi
Copy link
Contributor

Yinimi commented Dec 16, 2024

Looks like the viz.js version on DiagrammeRsvg is even more outdated

@Yinimi
Copy link
Contributor

Yinimi commented Dec 17, 2024

I updated the version in DiagrammeRsvg but I get an error which states atob is undefined.
@rich-iannone do you know where this is coming from in DiagrammeR? There it seems to be defined somehow.
I think the viz.js needs that atob function but I dont know how to include it properly.

@krlmlr
Copy link
Author

krlmlr commented Jan 8, 2025

I still see CI/CD failures: https://github.com/cynkra/dm/actions/runs/12661699082/job/35285626102#step:9:305 .

@Yinimi: would you like to share your DiagrammeRsvg updates?

@Yinimi
Copy link
Contributor

Yinimi commented Jan 8, 2025

I created a commit with the update of viz.js on this fork
https://github.com/Yinimi/DiagrammeR
But as said the viz.js needs this atob function to get it run.

@rich-iannone
Copy link
Owner

I had a closer look at how to make DiagrammeRsvg work (through export_graph()) for the update in viz.js. Seems pretty difficult (to me, at least) since the JS library doesn't seem to have a top-level Viz() function that can be called by V8.

I'd forgotten that the test suite (here in DiagrammeR) doesn't check for SVG output so merging wasn't the right thing to do (without separate tests of DiagrammeRsvg).

@Yinimi is it okay if I revert this until there's a more complete solution on your end? You could always resubmit new PRs. I know it's not ideal having such an old version of Graphviz here but it at least works well enough for those that depend on it.

@olivroy
Copy link
Collaborator

olivroy commented Jan 9, 2025

From Viz.js 2.0.0 changelog

The single Viz() function is now a class and reuses its Emscripten module instance, improving performance across multiple calls.

This was already identified in a previous DiagrammeR issue #314

More info where deprecation of Viz() is discussed in mdaines/viz-js#113

Deprecate the Viz() function in favor of format-specific rendering functions under the Viz namespace.

A PR that seems to try to update to Viz 2.0 PhE/jupyterlab_graphviz#8

https://github.com/embarklabs/embark/pull/2323/files

no0x9d/dependencies2graph@99b3a88

DoctorBud/graphviz-viewer@6694abb

I don't know much about JavaScript, but these seemed helpful!

Viz() seems to be replaced by viz.renderString()

In short, these lines would need to be updated.

https://github.com/rich-iannone/DiagrammeRsvg/blob/5057edac4947e38a41e9d76ab439b537b2e9d35f/R/export_svg.R#L42-L46

DiagrammeR and DiagrammeRsvg would need an update to CRAN at the same time.

@Yinimi
Copy link
Contributor

Yinimi commented Jan 9, 2025

@krlmlr sorry previously I accidentially added a link to my changes in DiagrammeR.
I commited some changes on this fork:
Yinimi/DiagrammeRsvg@219a4b4
Unfortunately I only know little about R and js. I am a bit confused why there is so many duplicated code in DiagrammeR and DiagrammeRsvg.

@rich-iannone I think I managed to add most of the required changes to the js code to call viz

For any reason the js environment seems to be different compared with how it is called in DiagrammeR.
There is this error:
! ReferenceError: atob is not defined
Its a bit ironic since the viz.js version on their download page is called viz-standalone.js

If anyone knows where to get that function from we might get it to run.
Otherwise reverting might be an option but I think we should try to use an updated version here

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

No branches or pull requests

4 participants