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

Introduce prettier to format JavaScript (.js) and Sass (.scss) files #594

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions .github/workflows/e2e-test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -94,12 +94,24 @@ jobs:
cd RhinoApp
Rscript ../test-lint-js.R

- name: format_js() should format JS scripts
if: always()
run: |
cd RhinoApp
Rscript ../test-format-js.R

- name: lint_sass() should detect lint errors in CSS
if: always()
run: |
cd RhinoApp
Rscript ../test-lint-sass.R

- name: format_sass() should format SASS (.scss) scripts
if: always()
run: |
cd RhinoApp
Rscript ../test-format-sass.R

- name: build_js() should build app.min.js
if: always()
run: |
Expand Down
2 changes: 1 addition & 1 deletion DESCRIPTION
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please bum the package version.

Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ BugReports: https://github.com/Appsilon/rhino/issues
License: LGPL-3
Encoding: UTF-8
Roxygen: list(markdown = TRUE)
RoxygenNote: 7.2.3
RoxygenNote: 7.3.1
Depends:
R (>= 2.10)
Imports:
Expand Down
2 changes: 2 additions & 0 deletions NAMESPACE
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@ export(app)
export(build_js)
export(build_sass)
export(diagnostics)
export(format_js)
export(format_r)
export(format_sass)
export(init)
export(lint_js)
export(lint_r)
Expand Down
9 changes: 8 additions & 1 deletion NEWS.md
Original file line number Diff line number Diff line change
@@ -1,4 +1,11 @@
# rhino 1.8.0
# rhino (development version)

1. Introduce `format_js()` and `format_sass()` powered by [prettier](https://prettier.io).
* **Note:** `lint_js()` and `lint_sass()` report styling errors.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add an entry for the migration guide (remove .rhino etc.)

They _might_ complain about formatting done with `format_js()` and `format_sass()` functions; however, we haven't spotted any issues so far.
If you face any problems with this, then please [raise an issue on GitHub](https://github.com/Appsilon/rhino/issues/new/choose)

# [rhino 1.8.0](https://github.com/Appsilon/rhino/releases/tag/v1.8.0)

1. All linter functions migrated to `box.linters`. New rhino projects will be configured to use linters from `box.linters`.
2. Updated GitHub Workflow template triggers.
Expand Down
46 changes: 46 additions & 0 deletions R/tools.R
Original file line number Diff line number Diff line change
Expand Up @@ -233,6 +233,29 @@ lint_js <- function(fix = FALSE) {
}
}

#' Format JavaScript
#'
#' Runs [prettier](https://prettier.io/) on JavaScript files in `app/js` directory.
#' Requires Node.js installed.
#'
#' You can prevent prettier from formatting a given chunk of your code by adding a special comment:
#' ```js
#' // prettier-ignore
#' ```
#' Read more about [ignoring code](https://prettier.io/docs/en/ignore).
#'
#' @param fix If `TRUE`, fixes formatting. If FALSE, reports formatting errors without fixing them.
#' @return None. This function is called for side effects.
#'
#' @export
format_js <- function(fix = TRUE) {
if (fix) {
npm("run", "format-js", "--", "--write")
} else {
npm("run", "format-js", "--", "--check")
}
}

#' Build Sass
#'
#' Builds the `app/styles/main.scss` file into `app/static/css/app.min.css`.
Expand Down Expand Up @@ -319,6 +342,29 @@ lint_sass <- function(fix = FALSE) {
}
}

#' Format Sass
#'
#' Runs [prettier](https://prettier.io/) on Sass (.scss) files in `app/styles` directory.
#' Requires Node.js installed.
#'
#' You can prevent prettier from formatting a given chunk of your code by adding a special comment:
#' ```scss
#' // prettier-ignore
#' ```
#' Read more about [ignoring code](https://prettier.io/docs/en/ignore).
#'
#' @param fix If `TRUE`, fixes formatting. If FALSE, reports formatting errors without fixing them.
#' @return None. This function is called for side effects.
#'
#' @export
format_sass <- function(fix = TRUE) {
if (fix) {
npm("run", "format-sass", "--", "--write")
} else {
npm("run", "format-sass", "--", "--check")
}
}

#' Run Cypress end-to-end tests
#'
#' Uses [Cypress](https://www.cypress.io/) to run end-to-end tests
Expand Down
2 changes: 1 addition & 1 deletion inst/WORDLIST
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
Addin
Addins
Appsilon
Boxifying
ESLint
Init
JS
Expand Down Expand Up @@ -53,6 +52,7 @@ renv
roxygen
rstudio
scalable
scss
shinyapps
shinymanager
shinytest
Expand Down
22 changes: 22 additions & 0 deletions inst/templates/node/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions inst/templates/node/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
"build-sass": "sass --no-source-map --style=compressed ../app/styles/main.scss:../app/static/css/app.min.css",
"lint-js": "eslint --config .eslintrc.json ../app/js",
"lint-sass": "stylelint ../app/styles",
"format-js": "prettier --config prettier.config.mjs --ignore-path none ../app/js/**/*.js",
"format-sass": "prettier --config prettier.config.mjs --ignore-path none ../app/styles/**/*.scss",
"run-app": "cd .. && Rscript -e \"shiny::runApp(port = 3333)\"",
"run-cypress": "cypress run --project ../tests",
"open-cypress": "cypress open --project ../tests",
Expand All @@ -22,6 +24,7 @@
"eslint-config-airbnb": "^19.0.4",
"eslint-import-resolver-webpack": "^0.13.8",
"eslint-plugin-import": "^2.29.1",
"prettier": "^3.3.2",
"sass": "^1.69.7",
"start-server-and-test": "^2.0.3",
"stylelint": "^14.16.1",
Expand Down
11 changes: 11 additions & 0 deletions inst/templates/node/prettier.config.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
/** @type {import("prettier").Config} */
export default {
overrides: [
{
files: "../app/js/**/*.js",
options: {
singleQuote: true,
},
},
],
};
26 changes: 26 additions & 0 deletions man/format_js.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

26 changes: 26 additions & 0 deletions man/format_sass.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions pkgdown/_pkgdown.yml
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,8 @@ reference:
- build_sass
- lint_js
- lint_sass
- format_js
- format_sass
- react_component

- title: Other tools
Expand Down
13 changes: 13 additions & 0 deletions tests/e2e/test-format-js.R
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
rhino::format_js()

# Create bad scripts and test if formatting returns the expected result
test_file_path <- fs::path("app", "js", "bad-style.js")
cat('const someFunction = (a ,b) => a+ b + "asdf"', file = test_file_path)
rhino::format_js()
testthat::expect_identical(
readLines(test_file_path),
"const someFunction = (a, b) => a + b + 'asdf';"
)

# Clean up
file.remove(test_file_path)
18 changes: 18 additions & 0 deletions tests/e2e/test-format-sass.R
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
rhino::format_sass()

# Create bad scripts and test if formatting returns the expected result
test_file_path <- fs::path("app", "styles", "bad-style.scss")
cat("@import 'asdf';\nx+y{ color: red}", file = test_file_path)
rhino::format_sass()
testthat::expect_identical(
readLines(test_file_path),
c(
'@import "asdf";',
"x + y {",
" color: red;",
"}"
)
)

# Clean up
file.remove(test_file_path)
Loading