Update structure for JS handling

[andrjohns]

Hey @wch, this PR updates the package to show a more efficient way of working with the JS library.

I've replaced the C code with a single bundled file for the SASS JS library (including instructions on bundling and patching from NPM), which gets loaded into a JSContext when the package is attached. This pre-loaded JSContext is then used for all sassy() calls. This means that you only need to pay the cost of loading the SASS library once, rather than for every function call.

Unfortunately this only works with the GitHub version of QuickJSR at the moment, since I had to add support for calling functions in modules. I'll be working on adding support for loading pre-compiled bytecode next, so I'll submit an update to CRAN once that's done.

Let me know if you have any questions!

Cheers, Andrew

[wch]

Excellent, thanks! Regarding the precompiled bytecode, is it possible to have QuickJSR compile JS to bytecode the first time it's used on a particular piece JS code, and then cache the bytecode?

My concern is if I were to precompile to bytecode with quickjs version X and save the bytecode it in this package, then later try to run the bytecode with quickjs version Y, that there would be a compatibility problem. If QuickJSR were to compile and cache the result (with the QuickJSR version as part of the cache key), then this worry would go away.

[andrjohns]

Excellent, thanks! Regarding the precompiled bytecode, is it possible to have QuickJSR compile JS to bytecode the first time it's used on a particular piece JS code, and then cache the bytecode?

I guess caching would be possible, but it would only be caching in the tempdir() - so there would only be a benefit for repeated evaluations in the same R session. I don't think there would be much benefit for sassy in particular - since the JS code only needs to be evaluated once for the entire session (unless the package is unloaded and reloaded).

[wch]

Question about this PR: does the technique you've used here, with browserify, allow the JS code access to the local filesystem? The technique that I had used was to take a build of sass that's meant for browsers, and then give it access to the quickjs os and std APIs to access the filesystem. Filesystem access is necessary because sometimes the .scss files import other .scss files.

https://github.com/wch/sass-quickjs/blob/main/browser-shims.js

[andrjohns]

Question about this PR: does the technique you've used here, with browserify, allow the JS code access to the local filesystem? The technique that I had used was to take a build of sass that's meant for browsers, and then give it access to the quickjs os and std APIs to access the filesystem. Filesystem access is necessary because sometimes the .scss files import other .scss files.

https://github.com/wch/sass-quickjs/blob/main/browser-shims.js

Oh good question, I'm not sure. Would you have a minimal example of these .scss nested imports that I could test with?

[wch]

A simple example is the file test.scss here: https://github.com/wch/sass-quickjs/tree/main/examples

I have also used it to compile Bootstrap 5 from here: https://github.com/twbs/bootstrap/tree/main/scss (However, I think the quickjs bug with the labels and loops causes problems with parsing comments and I think my workaround missed some instances, so the resulting .css has problems with comment blocks.)

[wch]

Also note that I think I had made more changes to the JS code to work around the QuickJS bug. See the two commits here: https://github.com/wch/sass-quickjs/commits/4cbc0fb45df350a680b41daa52441583ceeb5fec/sass.dart.js

[andrjohns]

A simple example is the file test.scss here: https://github.com/wch/sass-quickjs/tree/main/examples

This was really helpful, it's helped me figure out how to handle module imports properly with QuickJSR!

I've updated the included JS process to use the same browserify process (with the additional patches from your other commits), but with the addition of your original function and shims. The nested includes from the examples directory worked without issue for me (make sure to install the latest github QuickJSR before you test).

Let me know if you run into any other issues

[wch]

This is great, thanks.

One more quick thought about caching: you could use the rappdirs package to find a place to store caching artifacts. That's what we do for the existing sass and shinylive R packages. I can understand if you don't want to do that in QuickJSR because it adds complexity and most JS code won't really benefit from it because most JS code is much smaller than the code here. It would be nice, though, if QuickJSR made available the tools for doing that caching -- like being able to compile JS to bytecode in a raw vector, and being able to give that bytecode to QuickJSR to run.

[wch]

One more thing. I just tested the performance of this code, compiling bootstrap.scss.

After merging these changes:

system.time({ z <- sass("bootstrap.scss") })
#>    user  system elapsed 
#>  16.627   0.052  16.866 

But when I run it with the commit just before the merge, it's about 3x as fast:

system.time({ z <- sass("bootstrap.scss") })
#>    user  system elapsed 
#>   5.618   0.037   5.726 

Any idea why it's so much slower now? One possibility that just occurred to me is that in the old code, the result was just getting printed to stdout, but in the new code, it's being accumulated into a string which is then returned in the R object.

[andrjohns]

I can only replicate a much smaller difference locally

repo HEAD:

> remotes::install_github("wch/sassy",force=TRUE)
> system.time({ z <- sassy::sass("bootstrap.scss") })
   user  system elapsed 
  6.816   0.018   6.844 

Commit before merge (restarting R session in between):

> remotes::install_github("wch/sassy@187e661")
> system.time({ z <- sassy::sass("bootstrap.scss") })
... printed css ...
   user  system elapsed 
  5.939   0.028   6.006

This is likely due to the performance difference between logging to console instead of returning an R object.

The new code returns a list with both the CSS string and the loadedUrls metada list. QuickJSR has to walk each element of the list to convert it to the corresponding R object.

For the bootstrap.scss file, this metadata is a list of 87 elements, where each element is a 7-element list:

> length(z$loadedUrls)
[1] 86

> z$loadedUrls[[1]]
$href
[1] "file:///Users/andrew/Git/sassy/bootstrap-scss/mixins/banner"
$protocol
[1] "file"
$hostname
[1] ""
$port
[1] ""
$pathname
[1] "/Users/andrew/Git/sassy/bootstrap-scss/mixins/banner"
$search
[1] ""
$hash
[1] ""

If the metadata isn't of interest, then it's an easy performance gain to modify the JS function to just return the css element instead of the entire object

[wch]

At first I wasn't able to reproduce your results -- even after running your commands, the new version was much slower. But then I remembered that in my ~/.R/Makevars I had set -O0 for CFLAGS and CXXFLAGS to help with debugging things in the past. After I commented those out, then I got results that were very close to yours.