From 514a762bde3773c242041f8e4fd2ee61a8624361 Mon Sep 17 00:00:00 2001 From: Kamil Zyla Date: Wed, 17 Jan 2024 15:40:20 +0100 Subject: [PATCH] refactor: Apply review suggestions --- R/node.R | 49 +++++++++---------- R/tools.R | 20 ++++---- inst/WORDLIST | 7 ++- .../node-js-javascript-and-sass-tools.Rmd | 6 +-- 4 files changed, 40 insertions(+), 42 deletions(-) diff --git a/R/node.R b/R/node.R index 866cded1..7f0fbae9 100644 --- a/R/node.R +++ b/R/node.R @@ -2,41 +2,36 @@ node_path <- function(...) { fs::path(".rhino", ...) } -# Run `npm`/`bun` command (assume node directory already exists in the project). -js_package_manager_raw <- function(..., status_ok = 0) { - command <- Sys.getenv("RHINO_NPM", "npm") - withr::with_dir(node_path(), { - status <- system2(command = command, args = c(...)) - }) - if (status != status_ok) { - cli::cli_abort("System command '{command}' exited with status {status}.") - } -} - -# Run `npm`/`bun` command (create node directory in the project if needed). -js_package_manager <- function(...) { - command <- Sys.getenv("RHINO_NPM", "npm") - display_name <- ifelse(command == "npm", "Node.js", command) +# Run `npm` or an alternative command specified by `RHINO_NPM`. +# If needed, copy over Node.js template and install dependencies. +npm <- function(...) { + npm_command <- Sys.getenv("RHINO_NPM", "npm") check_system_dependency( - cmd = command, - dependency_name = display_name, + cmd = npm_command, + dependency_name = ifelse(npm_command == "npm", "Node.js", npm_command), documentation_url = "https://go.appsilon.com/rhino-system-dependencies" ) - init_js_package_manager() - js_package_manager_raw(...) + node_init(npm_command) + node_run(npm_command, ...) } -init_js_package_manager <- function() { - command <- Sys.getenv("RHINO_NPM", "npm") +node_init <- function(npm_command) { if (!fs::dir_exists(node_path())) { - message("Initializing Javascript configs\u2026") + cli::cli_alert_info("Initializing Node.js directory...") copy_template("node", node_path()) } - - # existing .rhino and missing node_modules folder - # indicate that packages were not installed but rhino project initialized if (!fs::dir_exists(node_path("node_modules"))) { - message("Installing dependencies by ", command, "\u2026") - js_package_manager_raw("install", "--no-audit", "--no-fund") + cli::cli_alert_info("Installing Node.js packages with {npm_command}...") + node_run(npm_command, "install", "--no-audit", "--no-fund") + } +} + +# Run the specified command in Node.js directory (assume it already exists). +node_run <- function(command, ..., status_ok = 0) { + withr::with_dir(node_path(), { + status <- system2(command = command, args = c(...)) + }) + if (status != status_ok) { + cli::cli_abort("System command '{command}' exited with status {status}.") } } diff --git a/R/tools.R b/R/tools.R index 4fef5111..859c2ca2 100644 --- a/R/tools.R +++ b/R/tools.R @@ -185,9 +185,9 @@ format_r <- function(paths) { #' @export build_js <- function(watch = FALSE) { if (watch) { - js_package_manager("run", "build-js", "--", "--watch", status_ok = 2) + npm("run", "build-js", "--", "--watch", status_ok = 2) } else { - js_package_manager("run", "build-js") + npm("run", "build-js") } } @@ -224,9 +224,9 @@ build_js <- function(watch = FALSE) { # nolint end lint_js <- function(fix = FALSE) { if (fix) { - js_package_manager("run", "lint-js", "--", "--fix") + npm("run", "lint-js", "--", "--fix") } else { - js_package_manager("run", "lint-js") + npm("run", "lint-js") } } @@ -277,9 +277,9 @@ build_sass <- function(watch = FALSE) { build_sass_node <- function(watch = FALSE) { if (watch) { - js_package_manager("run", "build-sass", "--", "--watch", status_ok = 2) + npm("run", "build-sass", "--", "--watch", status_ok = 2) } else { - js_package_manager("run", "build-sass") + npm("run", "build-sass") } } @@ -310,9 +310,9 @@ build_sass_r <- function() { #' @export lint_sass <- function(fix = FALSE) { if (fix) { - js_package_manager("run", "lint-sass", "--", "--fix") + npm("run", "lint-sass", "--", "--fix") } else { - js_package_manager("run", "lint-sass") + npm("run", "lint-sass") } } @@ -343,8 +343,8 @@ lint_sass <- function(fix = FALSE) { #' @export test_e2e <- function(interactive = FALSE) { if (interactive) { - js_package_manager("run", "test-e2e-interactive") + npm("run", "test-e2e-interactive") } else { - js_package_manager("run", "test-e2e") + npm("run", "test-e2e") } } diff --git a/inst/WORDLIST b/inst/WORDLIST index 70df777b..c89b7b65 100644 --- a/inst/WORDLIST +++ b/inst/WORDLIST @@ -1,3 +1,5 @@ +Addin +Addins Appsilon Boxifying ESLint @@ -13,9 +15,11 @@ Renv Renviron Rhinoverse Rprofile +Rstudio SDK Stylelint UI +Webpack blogpost conf config @@ -41,6 +45,7 @@ nodejs npm nvm overridable +pnpm preconfigured renv roxygen @@ -54,5 +59,3 @@ unintuitive usethis webpack yml -Addin -Addins diff --git a/vignettes/explanation/node-js-javascript-and-sass-tools.Rmd b/vignettes/explanation/node-js-javascript-and-sass-tools.Rmd index 1adeed9e..b896e43d 100644 --- a/vignettes/explanation/node-js-javascript-and-sass-tools.Rmd +++ b/vignettes/explanation/node-js-javascript-and-sass-tools.Rmd @@ -18,8 +18,8 @@ virtually any JavaScript library. You can use other package managers such as `npm`. To switch from the default npm usage, set a global environment variable named -`RHINO_NPM`. For instance, if you want to use `bun` instead of `npm`, enter -`RHINO_NPM=bun`. +`RHINO_NPM`. For instance, if you want to use `bun` instead of `npm`, +add `export RHINO_NPM=bun` to your shell startup file (e.g. `.bashrc`). Rhino uses Node.js to provide state of the art tools for working with JavaScript and Sass. The following functions require Node.js to work: @@ -32,7 +32,7 @@ JavaScript and Sass. The following functions require Node.js to work: ### Node directory -Under the hood Rhino will create a `.rhino/node` directory in your +Under the hood Rhino will create a `.rhino` directory in your project to store the specific libraries needed by these tools. This directory is git-ignored by default and safe to remove.