Best practices for packages registering engines?

[MichaelChirico]

Hello,

We are working through an issue in {lintr}, which processes Rmd files and needs to parse R chunks but ignore non-R chunks.

The issue is how to handle generic .Rmd files which may be using custom chunk engines. The issue specifically arose in relation to {tufte}'s marginfigure engine. Some more details are in an issue filed with {tufte}: https://github.com/rstudio/tufte/issues/117

Basically, it seems natural to me that packages offering custom engines should register them during .onLoad(), that way meta-tooling packages like {lintr} can "discover" them without having to execute any package code, or rely on certain things being exported, and can even avoid needing to include such packages as dependencies.

However, in the two packages that we've looked at so far, {bookdown} and {tufte}, the former does do such registration, but the latter doesn't. Further, the {knitr} documentation I found doesn't go into any recommendations/best practices:

https://bookdown.org/yihui/rmarkdown-cookbook/custom-engine.html

Now we are wondering if we are understanding the full picture, or if we are on to something that such use of .onLoad() is indeed a good recommendation?

[yihui]

Personally I don't have a strong preference or recommendation. Pre-registering engines in .onLoad() can definitely make the engines more easily discoverable. Registering them on-demand also has tiny benefits. I'd leave this decision to package authors.

[cderv]

I can add that a package registering an engine may not be a package only to be used with knitr, and sometimes knitr is only a suggest. So running knitr::knit_engine$set() in .onLoad() will not always be done when the package is loaded in a session.

There was an issue in glue about that, and engine is now registered conditionnaly to knitr being loaded https://github.com/tidyverse/glue/blob/main/R/zzz.R#L14-L21 Issue https://github.com/tidyverse/glue/issues/253 solved by https://github.com/tidyverse/glue/pull/254

I want to give this precision as it may impact the way you are doing things in lintr.

[MichaelChirico]

Thanks... I think loadNamespace("knitr") will suffice for us

Registering them on-demand also has tiny benefits.

Could you elaborate on that?

Or perhaps, just adding a slightly-fleshed out version of your comment to the documentation would suffice, maybe in a subsection like "Registering your engine from a package" presenting the different approaches/tradeoffs.

[yihui]

@MichaelChirico The decision is basically whether to register the engine conditionally or unconditionally. On-demand registration means the engine won't be available until it's actually to be used. The tiny benefits include: 1) it saves memory, albeit often trivial; 2) it avoids hard dependency on knitr, as mentioned by @cderv above.

I feel this is not specific to knitr engines, but a general decision to be made by package authors: when to call a function or use an object from another package. Doing it either early or late is fine. A very similar situation is whether to import a function from another package statically at build time, or call the function dynamically when it's actually needed. The pros and cons are often not that significant, so I feel it's probably not worth offering a guideline...