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

Defer JavaScript scripts in html_document #2046

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

Conversation

atusy
Copy link
Contributor

@atusy atusy commented Feb 14, 2021

This PR defers JavaScript scripts that are currently called as DOMContentLoaded event handlers.

Although this works fine in static html_document, it still fails in runtime: shiny.
This is probably because src is converted wrongly.
See highlighted line and the last line in the following image.

image

Reprex

---
title: "My First R Markdown Document" 
output:
  html_document:
    anchor_sections: true
    self_contained: false
runtime: shiny
---

# fooo

(closes #2032)

@yihui
Copy link
Member

yihui commented May 18, 2021

This is probably because src is converted wrongly.

<script src="header-attrs-2.6.6/%5Bobject%20Object%5D"></script>

is this when decoded:

<script src="header-attrs-2.6.6/[object Object]"></script>

It seems when the defer attribute is present, the src value is calculated by adding the string "header-attrs-..." to an object like

> "header-attrs-.../" + { "src": "foo.js", "defer": null}

"header-attrs-.../[object Object]"

I don't know where this string concatenation occurs. We will need help from the Shiny team (perhaps @cpsievert).

@cpsievert
Copy link
Contributor

cpsievert commented May 18, 2021

I'm pretty sure the script rendering issue is essentially a duplicate of rstudio/shiny#3345

I'll try to have a fix up by the end of the week

@cpsievert
Copy link
Contributor

cpsievert commented May 20, 2021

Ok, rstudio/shiny#3345 should be fixed on the main branch now, so the <script defer> approach shouldn't be a blocker anymore.

By the way, it's not immediately obvious to me that defer is the best way to solve this issue -- you might want to consider using setTimeout() instead since that's closer to what we do elsewhere to address these sort of timing issues

@atusy
Copy link
Contributor Author

atusy commented May 23, 2021

@cpsievert Thanks a lot!

setTimeout()

Would you tell me some background of this suggestion?
I think it is quite difficult to predict the adequate value for it.

@atusy
Copy link
Contributor Author

atusy commented May 23, 2021

Maybe the best way is to

  1. check if document.readyState is complete
    • this is what happens by runtime: shiny
  2. if true, run function immediately or use setTimeout
  3. otherwise, addEventListener as usual.

I wonder if there is an easy way to handle this for developers of custom formats or HTML dependencies.
It would be nice if they even do not have to think about it.
Maybe defer is not the best, but the quickest way for them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Backlog
Development

Successfully merging this pull request may close these issues.

Some JS based features does not load correctly in Shiny runtime document
3 participants