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

extra_dependencies order in bookdown::gitbook #1101

Closed
2 tasks done
linogaliana opened this issue Mar 16, 2021 · 15 comments · Fixed by #1249
Closed
2 tasks done

extra_dependencies order in bookdown::gitbook #1101

linogaliana opened this issue Mar 16, 2021 · 15 comments · Fixed by #1249
Labels
bug an unexpected problem or unintended behavior next to consider for next release
Milestone

Comments

@linogaliana
Copy link

linogaliana commented Mar 16, 2021

Context

I am trying to make a custom output format that adds a few features to the default gitbook output (repository here). The examples provided here all come from Github Actions build from this repository

Among the things I do, I add some extra_dependencies using some custom CSS and JS files (I have very limited knowledge in these two languages. I hope I will not state the obvious). This is there. However, when looking at the source code of the generated HTML (an example deployed with Netlify here ), I see that the order is

<link href="libs/utilitr-default-0.1.0/css/default.css" rel="stylesheet" />
<link href="libs/utilitr-default-0.1.0/css/style-utilitr.css" rel="stylesheet" />
<link href="libs/utilitr-default-0.1.0/css/icones-fa.css" rel="stylesheet" />
<link href="libs/utilitr-default-0.1.0/css/customize.css" rel="stylesheet" />
<script src="libs/utilitr-default-0.1.0/js/book.js"></script>
<script src="libs/jquery-2.2.3/jquery.min.js"></script>
<link href="libs/gitbook-2.6.7/css/style.css" rel="stylesheet" />
<link href="libs/gitbook-2.6.7/css/plugin-table.css" rel="stylesheet" />
<link href="libs/gitbook-2.6.7/css/plugin-bookdown.css" rel="stylesheet" />
<link href="libs/gitbook-2.6.7/css/plugin-highlight.css" rel="stylesheet" />
<link href="libs/gitbook-2.6.7/css/plugin-search.css" rel="stylesheet" />
<link href="libs/gitbook-2.6.7/css/plugin-fontsettings.css" rel="stylesheet" />
<link href="libs/gitbook-2.6.7/css/plugin-clipboard.css" rel="stylesheet" />

this means that some elements in our stylesheets might be overriden by elements defined in some stylesheets included in the bookdown package. I made an example there where I put some elements to define toc appearance in a CSS that is called in extra_dependencies (code there). Customized elements, e.g. text color in the TOC, are ignored (when looking at the output in Firefox).

The fix I found to force rendering is to put some elements in a style.css stylesheet called in the _output.yml. I used this fix to generate the aforementioned webpage where I modify the appareance of the TOC.

In my opinion, using the _output.yml file is not ideal when developping a format within a package. This makes it complicated, for instance, to modify the behavior of the gitbook table of content within the package (one cannot assume that the user of the package always add a CSS in the _output.yml YAML).

Why it matters ? (in my opinion, I am open to discussion)

I think the order of the dependencies matter because, when using a custom output format, one would expect the classes or properties defined in a few custom CSS to override some default bookdown/gitbook classes. Some attributes could still be set as !important but I am not sure this would always work.

A potential solution ?

I think a partial solution would be to change the order of extra_dependencies in bookdown::gitbook (here) like this is done in bookdown::bs4_book (here):

extra_dependencies = c(gitbook_dependency(table_css), extra_dependencies)

Regarding scripts in extra_dependencies

However, I think this does not solve everything regarding JS. Some scripts are present at two locations in the generated HTML. When matching the following pattern, scripts are copied at the bottom of the HTML (source here):

i = grep('^\\s*<script src=".+/gitbook([^/]+)?/js/[.a-z-]+[.]js"></script>\\s*$', head)

The scripts that I would define in extra_dependencies will not be duplicated (unless adopting a naming convention, which might be fine). This means you cannot override some functions if you want to change, let's say, the toolbar at the top.

Extra

Before posting the issue, I have followed the recommendations ;

  • formatted your issue so it is easier for us to read?

  • included a minimal, self-contained, and reproducible example?

@cderv
Copy link
Collaborator

cderv commented Mar 17, 2021

Thanks for the report @linogaliana !

I agree it would be more intuitive to have the extra dependencies sets after gitbook's one in

bookdown/R/gitbook.R

Lines 28 to 31 in 2b7830f

html_document2 = function(..., extra_dependencies = list()) {
rmarkdown::html_document(
..., extra_dependencies = c(extra_dependencies, gitbook_dependency(table_css))
)

because it is not easy to know what will be the effect of Gitbook deps on the extra deps, but the otherway around is usually known.

However, doing so could maybe break some existing book 🤔 @yihui some thoughts on this ?

The fix I found to force rendering is to put some elements in a style.css stylesheet called in the _output.yml. I used this fix to generate the aforementioned webpage where I modify the appareance of the TOC.
In my opinion, using the _output.yml file is not ideal when developping a format within a package. This makes it complicated, for instance, to modify the behavior of the gitbook table of content within the package (one cannot assume that the user of the package always add a CSS in the _output.yml YAML).

You can provide the css in you special format. You have a css file in your package, you set it as css argument in gitbook, and append to any user specify style ?

html_document <- function(extra_dependencies = list(),
                         css = NULL, ...) {
  extra_dependencies <- c(
    pkg_resource('rmarkdown/templates/utilitr/skeleton/style.css'),
    extra_dependencies
  )

  css <- c("utilitr.css", css)

  bookdown::gitbook(extra_dependencies = extra_dependencies,
                    css = css, ...)

}

did that not work ? I believe it should.
CSS file will be inserted after the extra dependencies. But I agree that setting CSS as a extra dependency using htmlDependency should work too.

However, I think this does not solve everything regarding JS. Some scripts are present at two locations in the generated HTML. When matching the following pattern, scripts are copied at the bottom of the HTML

The scripts are moved to bottom not copied, so they are not duplicated. But it indeed mean they will apply after your custom dependencies.

What exactly do you want to override or remove in the template ? Are you willing to keep gitbook feature ? you can also create your HTML template from scratch if you don't want gitbook feature.

I am trying to better understand the use case and the need to see if there are some more flexibility in customisation that we could provide. For example, you can already deactivate gitbook table css if you want to use bootstrap one.

Thanks for this by all means - it is very nice to know better the experience for a custom template. If it is not smooth enough, we can look into how to improve it.

@yihui
Copy link
Member

yihui commented Mar 24, 2021

However, doing so could maybe break some existing book 🤔 @yihui some thoughts on this ?

@cderv If swapping extra_dependencies and gitbook_dependency(table_css) solves the problem, I'm okay with doing that. I don't remember why I made the gitbook dependency the last in the list now, but I guess in practice, few people actually use the extra_dependencies argument of bookdown::gitbook(), so such a change might be fine.

The JS files need to be moved to the end of the HTML document (unless we use something like an onload event of the document). If you needs to execute JS code after the gitbook scripts, one hack is to name the dependencies to start with gitbook, e.g., gitbook-utilitr, so your scripts will be moved together with gitbook's.

P.S. I didn't read all the links mentioned in the above issue, so I don't fully understand what @linogaliana was trying to achieve.

@linogaliana
Copy link
Author

Thanks a lot to both of you for your answers.

Regarding my use case, I think I will move to bs4_book theme since it offers almost all functionalities I had in mind (except a print button that makes a nice paged HTML using paged.js but we are currently developping that with @oliviermeslin and @RLesur). Regarding my recent experience, bs4 model seems a little bit easier to customize so might be easier to start from as a template.

Regarding the issue, I think this would be more consistent to swap the order of dependencies but you know better the breaking changes this could bring. Regarding the JS files moved to the end, I think the naming convention proposed by @yihui is fine (this was the hack I had in mind).

@cderv cderv added bug an unexpected problem or unintended behavior next to consider for next release labels Mar 31, 2021
@cderv
Copy link
Collaborator

cderv commented Mar 31, 2021

If swapping extra_dependencies and gitbook_dependency(table_css) solves the problem, I'm okay with doing that. I don't remember why I made the gitbook dependency the last in the list now, but I guess in practice, few people actually use the extra_dependencies argument of bookdown::gitbook(), so such a change might be fine.

Yes I think switching could worth it and I don't think this should be harmful.

Regarding my recent experience, bs4 model seems a little bit easier to customize so might be easier to start from as a template.

Yes bs4_book() could be easier to customize using all the bslib and bootstrap features. However, be aware that this is a new format very recent and it could evolve quickly.

except a print button that makes a nice paged HTML using paged.js

This is a project I have in mind too. If you finish it and you agree to contribute, it would be a nice addition.
I had in mind a generic solution that you could use in several output format (and not just bookdown).
I am very interesting in this project, it does not surprise me that @RLesur is onto it! 😉

@oliviermeslin
Copy link

oliviermeslin commented Mar 31, 2021

@cderv : we have just finished implementing the print button in the bs4 model. The button itself is written in html. The pagination is done using the paged.js polyfill, customized CSS specifications from pagedown and a html header to call the polyfill.

All the codes are available in the utilitr package (here). The package contains a (long) MWE. Here is how to use this example:

  • install the package;
  • run the tests/testthat/test-manual.R script;
  • run servr::httd(), open the server in your browser, and click on _bs4_public;
  • the print button should appear at the top of the page;
  • click on it, you should get a paged html page after a few seconds.

Tell me if I can be of any help in the development of a more general solution.

@cderv
Copy link
Collaborator

cderv commented Mar 31, 2021

I opened a new issue to track this : #1117
We could discuss this there. I replace all your comment in there ! thanks !

@ThierryO
Copy link
Contributor

ThierryO commented Sep 1, 2021

Another idea would be to replace

  html_document2 = function(..., extra_dependencies = list()) {
    rmarkdown::html_document(
      ..., extra_dependencies = c(extra_dependencies, gitbook_dependency(table_css))
    )
  }

with

  html_document2 = function(..., extra_dependencies = list(), important_dependencies = list()) {
    rmarkdown::html_document(
      ..., extra_dependencies = c(extra_dependencies, gitbook_dependency(table_css), important_dependencies)
    )
  }

This won't break the old behaviour. And it gives users the choice wheter to import the dependencies before and/or after the gitbook dependencies.

@cderv
Copy link
Collaborator

cderv commented Sep 1, 2021

This would introduce a new argument and I don't like that. I don't think this is necessary. I don't see why gitbook deps needs to be after extra_dependencies so we'll change that.

@cderv
Copy link
Collaborator

cderv commented Sep 1, 2021

@ThierryO @linogaliana can you test #1249 on your projects and see if you encounter any obvious weird behavior ?

remotes::install_github("rstudio/bookdown#1249")
# or 
pak::pak("rstudio/bookdown#1249")
# or any other function of your choice.
```

Thank you! 

@ThierryO
Copy link
Contributor

ThierryO commented Sep 1, 2021

I get an error Error in system.file(..., package = "bookdown", mustWork = TRUE) : no file found

Here is the relevant part of the traceback

24: stop("no file found")
23: system.file(..., package = "bookdown", mustWork = TRUE) at utils.R#4
22: bookdown_file("resources", "jquery") at addins.R#2
21: htmltools::htmlDependency("jquery", "2.2.3", bookdown_file("resources", 
        "jquery"), script = "jquery.min.js") at addins.R#2
20: jquery_dependency() at gitbook.R#91
19: gitbook_dependency(table_css) at gitbook.R#29
sessioninfo::session_info("bookdown")
─ Session info ───────────────────────────────────────────────────────────────────────────────────────────────────────────
 setting  value                       
 version  R version 4.1.1 (2021-08-10)
 os       Ubuntu 18.04.5 LTS          
 system   x86_64, linux-gnu           
 ui       RStudio                     
 language nl_BE:nl                    
 collate  nl_BE.UTF-8                 
 ctype    nl_BE.UTF-8                 
 tz       Europe/Brussels             
 date     2021-09-01                  

─ Packages ───────────────────────────────────────────────────────────────────────────────────────────────────────────────
 ! package   * version date       lib source                           
   base64enc   0.1-3   2015-07-28 [1] CRAN (R 4.1.0)                   
 V bookdown    0.22    2021-09-01 [1] Github (rstudio/bookdown@780b80e)
   digest      0.6.27  2020-10-24 [1] CRAN (R 4.1.0)                   
   evaluate    0.14    2019-05-28 [1] CRAN (R 4.1.0)                   
   fastmap     1.1.0   2021-01-25 [1] CRAN (R 4.1.0)                   
   glue        1.4.2   2020-08-27 [1] CRAN (R 4.1.0)                   
   highr       0.9     2021-04-16 [1] CRAN (R 4.1.0)                   
 V htmltools   0.5.1.1 2021-08-25 [1] CRAN (R 4.1.1)                   
   jquerylib   0.1.4   2021-04-26 [1] CRAN (R 4.1.0)                   
   jsonlite    1.7.2   2020-12-09 [1] CRAN (R 4.1.0)                   
   knitr     * 1.33    2021-04-24 [1] CRAN (R 4.1.0)                   
   magrittr    2.0.1   2020-11-17 [1] CRAN (R 4.1.0)                   
   markdown    1.1     2019-08-07 [1] CRAN (R 4.1.0)                   
   mime        0.11    2021-06-23 [1] CRAN (R 4.1.0)                   
   rlang       0.4.11  2021-04-30 [1] CRAN (R 4.1.0)                   
   rmarkdown   2.10    2021-08-06 [1] CRAN (R 4.1.1)                   
   stringi     1.7.4   2021-08-25 [1] CRAN (R 4.1.1)                   
   stringr     1.4.0   2019-02-10 [1] CRAN (R 4.1.0)                   
   tinytex     0.33.1  2021-09-01 [1] Github (yihui/tinytex@0aaac57)   
 V xfun        0.24    2021-08-06 [1] CRAN (R 4.1.1)                   
   yaml        2.2.1   2020-02-01 [1] CRAN (R 4.1.0)                   

[1] /home/thierry/R/x86_64-pc-linux-gnu-library/4.0
[2] /usr/local/lib/R/site-library
[3] /usr/lib/R/site-library
[4] /usr/lib/R/library

 V ── Loaded and on-disk version mismatch.

@cderv
Copy link
Collaborator

cderv commented Sep 1, 2021

I am puzzled about the version of bookdown installed and used. This error means that an incorrect version of bookdown is used. last CRAN version of bookdown is now using jquerylib and the PR also.

So, if I may, something is wrong with how you are testing. See the V in front of bookdown and the note

 V ── Loaded and on-disk version mismatch.

Can you try reinstalling from the PR and restart your session before retesting ? You need to restart after an installation usually.

@ThierryO
Copy link
Contributor

ThierryO commented Sep 1, 2021

I reinstalled the new version. Now it works. The stylesheets from my custom extra_dependencies are listed between the gitbook dependencies and those from packages I use in the code (e.g. DT, crosstalk, ...)

Rscript -e 'sessioninfo::session_info("bookdown")'
─ Session info ───────────────────────────────────────────────────────────────
 setting  value                       
 version  R version 4.1.1 (2021-08-10)
 os       Ubuntu 18.04.5 LTS          
 system   x86_64, linux-gnu           
 ui       X11                         
 language nl_BE:nl                    
 collate  nl_BE.UTF-8                 
 ctype    nl_BE.UTF-8                 
 tz       Europe/Brussels             
 date     2021-09-01                  

─ Packages ───────────────────────────────────────────────────────────────────
 package   * version date       lib source                           
 base64enc   0.1-3   2015-07-28 [1] CRAN (R 4.1.0)                   
 bookdown    0.23.4  2021-09-01 [1] Github (rstudio/bookdown@49ea95a)
 digest      0.6.27  2020-10-24 [1] CRAN (R 4.1.0)                   
 evaluate    0.14    2019-05-28 [1] CRAN (R 4.1.0)                   
 fastmap     1.1.0   2021-01-25 [1] CRAN (R 4.1.0)                   
 glue        1.4.2   2020-08-27 [1] CRAN (R 4.1.0)                   
 highr       0.9     2021-04-16 [1] CRAN (R 4.1.0)                   
 htmltools   0.5.2   2021-08-25 [1] CRAN (R 4.1.1)                   
 jquerylib   0.1.4   2021-04-26 [1] CRAN (R 4.1.0)                   
 jsonlite    1.7.2   2020-12-09 [1] CRAN (R 4.1.0)                   
 knitr       1.33    2021-04-24 [1] CRAN (R 4.1.0)                   
 magrittr    2.0.1   2020-11-17 [1] CRAN (R 4.1.0)                   
 markdown    1.1     2019-08-07 [1] CRAN (R 4.1.0)                   
 mime        0.11    2021-06-23 [1] CRAN (R 4.1.0)                   
 rlang       0.4.11  2021-04-30 [1] CRAN (R 4.1.0)                   
 rmarkdown   2.10    2021-08-06 [1] CRAN (R 4.1.1)                   
 stringi     1.7.4   2021-08-25 [1] CRAN (R 4.1.1)                   
 stringr     1.4.0   2019-02-10 [1] CRAN (R 4.1.0)                   
 tinytex     0.33.1  2021-09-01 [1] Github (yihui/tinytex@0aaac57)   
 xfun        0.25    2021-08-06 [1] CRAN (R 4.1.1)                   
 yaml        2.2.1   2020-02-01 [1] CRAN (R 4.1.0)                   

[1] /home/thierry/R/x86_64-pc-linux-gnu-library/4.0
[2] /usr/local/lib/R/site-library
[3] /usr/lib/R/site-library

@cderv
Copy link
Collaborator

cderv commented Sep 1, 2021

Now it works.

Great ! If you see no obvious problem with how gitbook() is working, then it should be fine.

I leave the PR open a bit, waiting for @linogaliana feedback so that we confirm than switching the order is ok.

@linogaliana do you have time to give a try too ?

@yihui
Copy link
Member

yihui commented Sep 1, 2021

I leave the PR open a bit, waiting for @linogaliana feedback so that we confirm than switching the order is ok.

I guess it should be good. I just merged the PR, and @linogaliana can test with remotes::install_github('rstudio/bookdown'). If anything is wrong, we can revert the PR. Thanks!

@cderv cderv added this to the v0.24 milestone Sep 1, 2021
@github-actions
Copy link

github-actions bot commented Mar 2, 2022

This old thread has been automatically locked. If you think you have found something related to this, please open a new issue by following the issue guide (https://yihui.org/issue/), and link to this old issue if necessary.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 2, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug an unexpected problem or unintended behavior next to consider for next release
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants