Skip to content

fix: move required Shiny app packages from Suggests to Imports#1209

Open
Gero1999 wants to merge 16 commits intomainfrom
1207-bug/fix-suggests-imports-classification
Open

fix: move required Shiny app packages from Suggests to Imports#1209
Gero1999 wants to merge 16 commits intomainfrom
1207-bug/fix-suggests-imports-classification

Conversation

@Gero1999
Copy link
Copy Markdown
Collaborator

@Gero1999 Gero1999 commented Apr 9, 2026

Issue

Closes #1207

Description

run_app() fails on a clean install because several packages required by the Shiny app are listed in Suggests instead of Imports. Users must manually install logger, reactable.extras, shinyjqui, and others before the app can start.

This PR reclassifies packages based on a full audit of usage across R/ and inst/shiny/:

Moved from Suggests → Imports (required at app startup, no fallback possible):

  • bslib, htmltools, htmlwidgets, logger
  • reactable, reactable.extras
  • shiny, shinycssloaders, shinyjs, shinyjqui, shinyWidgets
  • zip

Removed entirely:

  • stringi — only used for random string generation (2 calls). Replaced with base R paste0(sample(c(letters, 0:9), 5, replace = TRUE), collapse = "").

Kept in Suggests (properly guarded with requireNamespace() or dev/test-only):

  • File readers: arrow, haven, readxl — guarded in R/readers.R
  • Export formats: officer, flextable — guarded in inst/shiny/functions/zip-utils.R
  • TLG: rlistings, ggh4x — guarded at point-of-use
  • Reporting: quarto — guarded in R/quarto-utils.R
  • Dev/test: covr, DT, jsonlite, knitr, lintr, markdown, mockery, nestcolor, pak, rmarkdown, sass, scales, shinytest2, testthat, vdiffr, withr

Other changes:

  • check_app_dependencies() now reads the Imports field from DESCRIPTION automatically instead of maintaining a hardcoded list.
  • Removed require(stringi) from inst/shiny/app.R.
  • Added R/imports-shiny.R with @importFrom directives for packages only used in inst/shiny/ (required by R CMD check).
  • Added tryCatch guard to sessionInfo() rendering in the About tab.
  • Added data-raw/test_suggests_hidden.R for manual dependency testing.
  • Added dependency test step to the PR checklist template.

Definition of Done

  • install.packages("aNCA") installs all packages needed for run_app() to work without additional steps.
  • Optional features (SAS import, PowerPoint export, etc.) degrade gracefully with informative messages when their packages are missing.

How to test

Use the manual test script in a fresh R session:

source("data-raw/test_suggests_hidden.R")

This hides all Suggests packages, loads the package, and launches run_app(). Verify:

  1. The app starts without errors.
  2. Core workflow works: upload data → map variables → run NCA → export ZIP.
  3. Optional features (SAS import, PowerPoint export, listings) show graceful messages instead of crashing.

After stopping the app, restore packages:

restore_packages()

Contributor checklist

  • Code passes lintr checks
  • Code passes all unit tests
  • New logic covered by unit tests
  • New logic is documented
  • App or package changes are reflected in NEWS
  • Package version is incremented
  • R script works with the new implementation (if applicable)
  • Settings upload works with the new implementation (if applicable)
  • If any .scss change was done, run data-raw/compile_css.R
  • If a package dependency was added/changed, run data-raw/test_suggests_hidden.R

Notes to reviewer

This is a dependency classification fix, not a behavior change. The app should function identically — the only difference is that install.packages("aNCA") now pulls in everything needed for run_app().

The stringi replacement uses base R's sample() which is sufficient for generating unique module IDs.

Version bump and NEWS entry still needed before merge.

@Gero1999 Gero1999 force-pushed the 1207-bug/fix-suggests-imports-classification branch 2 times, most recently from ed8b5e1 to c468a8a Compare April 9, 2026 11:00
Move packages that are required for run_app() from Suggests to Imports
so they are installed automatically with install.packages('aNCA').

Moved to Imports: bslib, htmltools, htmlwidgets, logger,
reactable, reactable.extras, shiny, shinycssloaders, shinyjs,
shinyjqui, shinyWidgets, zip.

Removed stringi dependency entirely, replaced with base R
paste0(sample()) for random string generation.

Simplified check_app_dependencies() since all core app packages
are now guaranteed by Imports.

Co-authored-by: Ona <no-reply@ona.com>
@Gero1999 Gero1999 force-pushed the 1207-bug/fix-suggests-imports-classification branch 2 times, most recently from 81ec0c6 to 5d22712 Compare April 9, 2026 11:11
later is always available when shiny is installed. No need to list
it explicitly in DESCRIPTION.

Co-authored-by: Ona <no-reply@ona.com>
Gero1999 and others added 6 commits April 10, 2026 11:28
Wrap sessionInfo() rendering in tryCatch so the About tab doesn't
crash when optional packages have missing or broken DESCRIPTION files.

Co-authored-by: Ona <no-reply@ona.com>
Adds data-raw/test_suggests_hidden.R which hides all Suggests packages
and launches run_app() to verify core functionality works without them.
Already excluded from the package build via .Rbuildignore.

Co-authored-by: Ona <no-reply@ona.com>
Added checklist item for running test_suggests_hidden.R when dependencies change.
Co-authored-by: Ona <no-reply@ona.com>
R CMD check requires every Imports package to have at least one
importFrom in NAMESPACE. These packages are used in inst/shiny/
which R CMD check does not scan.

Co-authored-by: Ona <no-reply@ona.com>
@Gero1999 Gero1999 requested review from Shaakon35 and js3110 April 10, 2026 10:47
@Gero1999 Gero1999 marked this pull request as ready for review April 13, 2026 15:17
Copy link
Copy Markdown
Collaborator

@js3110 js3110 left a comment

Choose a reason for hiding this comment

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

Hey, looks good. I am getting two additional warning messages when I run with the hidden pacakges:

Warning in FUN(X[[i]], ...) :
  DESCRIPTION file of package 'shinytest2' is missing or broken

Warning message:
In read.dcf(file.path(p, "DESCRIPTION"), c("Package", "Version")) :
  cannot open compressed file '/AppData/Local/R/win-library/4.4/knitr/DESCRIPTION', probable reason 'No such file or directory'

Comment thread data-raw/test_suggests_hidden.R
@Gero1999
Copy link
Copy Markdown
Collaborator Author

Hey, looks good. I am getting two additional warning messages when I run with the hidden pacakges:

Warning in FUN(X[[i]], ...) :
  DESCRIPTION file of package 'shinytest2' is missing or broken

This type of messages are for now expected, should not be a problem in the future

Warning message:
In read.dcf(file.path(p, "DESCRIPTION"), c("Package", "Version")) :
cannot open compressed file '/AppData/Local/R/win-library/4.4/knitr/DESCRIPTION', probable reason 'No such file or directory'

@js3110 This one on the other hand I am not completely clear why it happens to you

@js3110
Copy link
Copy Markdown
Collaborator

js3110 commented Apr 14, 2026

ok @Gero1999 , idk either but everyting still works anyway, so I guess the warning messages can be ignored

Copy link
Copy Markdown
Collaborator

@js3110 js3110 left a comment

Choose a reason for hiding this comment

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

LGTM :)

@Gero1999 Gero1999 requested a review from bachapman April 16, 2026 07:36
@Gero1999
Copy link
Copy Markdown
Collaborator Author

@billdenney feel free to check this PR if you have time

@Shaakon35
Copy link
Copy Markdown
Collaborator

Shaakon35 commented Apr 17, 2026

@Gero1999 It does not work for me, I followed the how to test instructions and downloaded the zip file with all, but it didn't work:
image

This bug of of downloading a htm file is annoying, we should fix this globally by a try catch...

Build session info manually with per-package tryCatch instead of
relying on print.sessionInfo(), which crashes when packages have
missing or broken DESCRIPTION files.

Co-authored-by: Ona <no-reply@ona.com>
Wrap sessionInfo() in .export_session_info() with tryCatch so a
package with a broken DESCRIPTION file does not prevent the entire
ZIP from being created.

Co-authored-by: Ona <no-reply@ona.com>
@Gero1999
Copy link
Copy Markdown
Collaborator Author

Gero1999 commented Apr 17, 2026

@Shaakon35 Some exportable objects (PPT, session_info) are indeed coming from Suggested packages so they have to be addressed within the ZIP. Hopefully now is fixed

the session_info.txt will still appear for the user, but it will have the error message that indicates that could not be done. I don't want to change it per ce because other branches are already modifying the way it behaves (as it might stop using the logger package)

Copy link
Copy Markdown
Collaborator

@bachapman bachapman left a comment

Choose a reason for hiding this comment

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

Works well for me!

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

Successfully merging this pull request may close these issues.

Bug: aNCA::run_app() requires packages in the Suggests list

4 participants