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

update core-js version in html_dependency_corejs() #85

Open
jhk0530 opened this issue May 24, 2024 · 2 comments
Open

update core-js version in html_dependency_corejs() #85

jhk0530 opened this issue May 24, 2024 · 2 comments

Comments

@jhk0530
Copy link

jhk0530 commented May 24, 2024

Hi, thanks for awesome work.

When reactR used in Quarto HTML page and commited to github.

This will cause security problem like below.

스크린샷 2024-05-24 오후 9 13 56

*note, above image says that issue closed (since I changed to not use reactR in that code)

To reproduce this, use below as contents of index.qmd and render with quarto. (Which is example from readme)

```{r}
library(reactR)
library(htmltools)

browsable(tagList(
  tags$div(id = "app"),
  tags$script(
  "
    ReactDOM.render(
      React.createElement(
        'h1',
        null,
        'Powered by React'
      ),
      document.getElementById('app')
    )
  "
  ),
  #add core-js first to work in RStudio Viewer
  html_dependency_corejs(),
  html_dependency_react()
))
```

Actually, used the core-js-2.5.3 version of the javascript library will cause this problem.

and the code

html_dependency_corejs()

which is actually works as below

htmltools::htmlDependency(name = "core-js", version = "2.5.3", 
        src = c(file = system.file("www/core-js/", package = "reactR")), 
        script = "shim.min.js")

cause this.

to solve this. updating version from 2.5.3 to further version which is not use grunt-karma as <=4.0.1 or latest(3.37.1) can be considered.

Note

I don't think core-js is required any more o to work in Rstudio viewer at now (2024)

Thanks.

@timelyportfolio
Copy link
Collaborator

timelyportfolio commented Jun 26, 2024

@jhk0530 Thanks so much. You are correct that core-js is no longer required, but I do feel like I should continue to include for anyone on legacy setups. I plan to push 0.6.0 to CRAN this week, but I am worried this might require testing that would delay this release. Over the next couple of weeks, I'll try to

  1. update core-js to core-js-bundle@3.37.1 which unfortunately is 229kb versus previous 85.9kb
  2. remove core-js from the default dependencies in the templates but any widgets and inputs built with prior templates will still by default include core-js. Updated core-js in step 1 should mean though that everything works.

@glin any thoughts or concerns?

@glin
Copy link
Contributor

glin commented Jun 28, 2024

@timelyportfolio No concerns, I doubt core-js is still necessary in >99% of cases. I had also wanted to remove core-js from reactable a few years ago during the IE11 EOL because of its added size, and that it was getting flagged for vulnerabilities (glin/reactable#245 (comment))

Removing it by default but leaving it in the package to opt into sounds like a good idea.

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

No branches or pull requests

3 participants