slowkow / ggrepel

:round_pushpin: Repel overlapping text labels away from each other in your ggplot2 figures.
https://ggrepel.slowkow.com
GNU General Public License v3.0
1.21k stars 95 forks source link

Remove dependency on `grid` for webR #256

Closed JZauner closed 7 months ago

JZauner commented 7 months ago

ggrepel is suprisingly absent from the WebR binary package repository (https://repo.r-wasm.org). Looking into the dependencies of ggrepel, this is likely due to the import grid, which is now part of base r (https://cran.r-project.org/web/packages/grid/index.html) and not a separate package. If this dependency could be removed as an explicit (and now unneeded) import/dependency, ggrepel (and downstream apps using it) will likely be available for webR, which would be great :)

slowkow commented 7 months ago

I'd love to have ggrepel included in the WebR repository.

But I'm not convinced that the grid dependency is the culprit. I found several other packages (ggpp, patchwork, ggbreak, ggwordcloud) that list grid in Imports: and all of them are present in the WebR repository.

(I tried deleting grid from Imports:, and to my surprise the package is still building just fine, and passing R CMD check without any warnings. So, I'm not sure whether or not grid needs to be listed in the DESCRIPTION file — I would be inclined to keep it there, because that's what all the other developers are doing.)

JZauner commented 7 months ago

makes total sense. Do you have other ideas why it might not be present?

slowkow commented 7 months ago

I don't know why, maybe the authors of WebR have some ideas?

Hey @georgestagg we are wondering if there is any reason why ggrepel is being excluded from WebR while other ggplot2 extensions are being included. Is it possible to make ggrepel compatible?

georgestagg commented 7 months ago

It looks like Rcpp/Benchmark/Timer.h doesn't work under Wasm - the OS detection in that file doesn’t know about Emscripten.

If possible, and acceptable to the Rcpp authors, the required change would be best made in that package so that it also applies to other packages using Rcpp. Or perhaps a change to the webR build tools so that the header does the right thing.

However, in case it is useful there is a workaround possible in src/repel_boxes.cpp.

- #include <Rcpp/Benchmark/Timer.h>
+ #ifdef __EMSCRIPTEN__
+ #define __linux
+ #include <Rcpp/Benchmark/Timer.h>
+ #undef __linux
+ #else
+ #include <Rcpp/Benchmark/Timer.h>
+ #endif

I tried the workaround and it seems to work. I'll make a note to revisit this for a more elegant solution when I can.


Screenshot 2024-02-10 at 15 38 33
slowkow commented 7 months ago

Amazing! I'll never get tired of seeing R packages work in the browser. Feels like magic every time.

Thank you so much for taking the time to find the problem, share the solution, write this up, test it, and share the output. I really appreciate your hard work here. And I'm sure a lot of other Rcpp users will appreciate your work, too. 🙏