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

Website in Viewer is updated before Rmd rendering #486

Closed
cderv opened this issue Oct 8, 2020 · 30 comments
Closed

Website in Viewer is updated before Rmd rendering #486

cderv opened this issue Oct 8, 2020 · 30 comments

Comments

@cderv
Copy link
Collaborator

cderv commented Oct 8, 2020

I believe with blogdown.knit.on_save activated, the viewer is reloaded before the Rmd is rerendered with the change. At least, in most of the time and on Windows.

This cause the viewer to not show the last change but the previous one.

Steps when this happens

See #481 for full workflow

  • Open a new blodown project with auto build activated
  • Hugo server starts on the homepage
  • Navigate in the viewer to the post you want to modify
  • Modify the Rmd file and Save
  • The viewer blink and does not show last change. However, the post is correctly rerendered
Rendering content/post/2015-07-23-r-rmarkdown.Rmd... Done.
  • Add another modification in the Rmd file and save
  • The viewer is updated but show the first change that was made and not the one we just did.
  • Manually reload the viewer - you are sent back to the homepage
  • Navigate to the post - change is there

Maybe related to --navigateToChanged not working ?

Let's note that it works ok if the post is opened in the browser

There is something not stable in this I believe. I'll look into it based on these notes.

@cderv
Copy link
Collaborator Author

cderv commented Oct 8, 2020

Out of curiosity I tried on Ubuntu with Rstudio Server and the website does do not update...

having the echo of stout hugo process helps see the website is correctly updated.

Maybe an issue with newer version of hugo ?

@cderv
Copy link
Collaborator Author

cderv commented Oct 8, 2020

Maybe an issue with newer version of hugo ?

So live reload is working well with last hugo version when I test without R

  • Launch a blogdown project (Default one)
  • Modify the lorem ipsum md post and save
  • The viewer is correctly updated and correctly show the modified page.

That means LiveReload is working well and --NavigateToChanged also.

This is just not working correctly with Rmd post, and I believe it is because of the issue you mentioned : All the files are watched by the hugo server (including intermediates files) and this is surely causing issue to reload to the correct file.

IMO this impact the experience while working in RStudio

@yihui
Copy link
Member

yihui commented Oct 8, 2020

I think this is mostly because build_rmds() moves files around in the project, which confuses Hugo and it fails to infer which page it should navigate to.

Using page bundles will make it much better. I was working on it yesterday, but didn't manage to finish it.

Another thing is that we have to ignore .knit.md and .utf8.md (e.g., yihui/hugo-xmin@f8510d8), otherwise Hugo may also try to render them. We'll need to document this.

The bug gohugoio/hugo#3811 that I mentioned may be irrelevant. It will be great if it could be fixed, but I don't think it's the culprit for the failure of --navigateToChanged.

@cderv
Copy link
Collaborator Author

cderv commented Oct 8, 2020

I think this is mostly because build_rmds() moves files around in the project, which confuses Hugo and it fails to infer which page it should navigate to.

This is indeed what I observe if I run the server hugo server in a Terminal and read what is printed as output when I modify the files. Lots of file watched and removes and written.

Another thing is that we have to ignore .knit.md and .utf8.md (e.g., yihui/hugo-xmin@f8510d8), otherwise Hugo may also try to render them. We'll need to document this.

Those are intermediary files that are temporary during the Rmd building right ? Is it a bad idea to set intermediate_dir in render() to a temp directory ?
Or one way to "trick" hugo would be to render everything in another folder and move back to where hugo is watching only when rendered. You may already have thought about that so again wrong idea ? 😅

It will be great if it could be fixed, but I don't think it's the culprit for the failure of --navigateToChanged.

I was under the impression that, as all the files are watched included the ignored file, --navigateToChanged won't know which one to navigate, and if those file were correctly ignored, that would leave us with the html file only. 🤷‍♂️

@yihui
Copy link
Member

yihui commented Oct 9, 2020

Those are intermediary files that are temporary during the Rmd building right ? Is it a bad idea to set intermediate_dir in render() to a temp directory ?

I'm not very comfortable with using intermediate_dir. You probably know that we have had a lot of bug reports related to this argument in the rmarkdown repo. On the other hand, ignoring .knit.md and .utf8.md is an easy task for users. On a related note, we really should retire the intermediate .knit.md and .utf8.md files in the rmarkdown package someday. They are no longer necessary now because we only support UTF-8, so for foo.Rmd, foo.knit.md and foo.utf8.md are identical to foo.md.

Or one way to "trick" hugo would be to render everything in another folder and move back to where hugo is watching only when rendered. You may already have thought about that so again wrong idea ?

The challenge is that you can't easily move an Rmd file away and render it elsewhere when it references external files (e.g., read.csv('../data/foo.csv')). Moving the Rmd file is probably less robust than using intermediate_dir.


As I said, with page bundles, this can be much easier. The _files and _cache directories don't need to be moved. The main decision that I'm still hesitating on is what to do with HTML dependencies. @apreshill suggested in #476 that I just leave them alone in the _files folder instead of moving them to static/ (of course, this requires the assumption of using page bundles), which is extremely easy for me (things can't be easier if you are told not to do anything), but I've been hesitating because there is a downside of this way, i.e. we may have a lot of duplicate CSS/JS libraries in a site. Some HTML dependencies are large and complex, such as those from DT and leaflet, and I don't feel very comfortable duplicating them for each post. This was the motivation behind moving HTML dependencies to static/rmarkdown-libs/.

Perhaps I should just provide an option and let users decide if they want to move HTML dependencies to static/. If they want to move, this will be another source of change for Hugo's livereload, which may also confuse Hugo's navigation (--navigateToChanged).

I'll keep experimenting. I probably can't make the auto navigation work perfectly for the coming CRAN release. One thing is for sure: the experience of writing pure Markdown post will be much much better. Knitting individual posts used to be a totally wrong action for users to do, and now at least pressing the Knit button won't hurt any more. I guess that's already a huge step forward, despite the annoyance of excessive auto-refreshing and lack of support for auto-navigation.

@yihui yihui closed this as completed in 04ef6ee Oct 9, 2020
@yihui
Copy link
Member

yihui commented Oct 9, 2020

I ended up using a dirty trick 04ef6ee to let Hugo know for sure which page it should navigate to. The trick was to wait for 2 seconds after building the Rmd file, and rewrite it one more time. It seems to work for both page bundles and non-bundles.

There is still unnecessary refreshing. I'll see if I can fix it tomorrow.

@cderv
Copy link
Collaborator Author

cderv commented Oct 9, 2020

Thanks for the detailed explanation above. It makes perfect sense.

Re. the dirty trick, this is clever !

Just tested it:

It seems to work well on Windows once you first navigated on the page. When opening the project, it will open the home page, the modifying and saving will trigger the render but it won't navigate to the post page as it does for .md file. However, once you are on the post page, it will live reload ok now! That is a lot better experience !

I also tested on RStudio server and this does not work there. I think it is specific to the server architecture. Opening devtools console I got a loading error
image

livereload.js:1 Failed to load resource: the server responded with a status of 404 (Not Found)

I believe the js file from the local hugo installation is not found or exposed correctly on Rstudio server situation. This is a limitation we should document if we don't find a workaround. I'll try something though...

@cderv
Copy link
Collaborator Author

cderv commented Oct 9, 2020

I'll try something though...

Ok it is not working as I thought. I wonder if some parameters should pass to hugo server when used on RStudio server

@cderv
Copy link
Collaborator Author

cderv commented Oct 9, 2020

And I think this is because RStudio server act as a proxy. There is even a function in rstudioapi to help with that. For example, if a hugo server is served on port 6380

> rstudioapi::translateLocalUrl("https://localhost:6380/livereload.js")
[1] "p/56a946ed/livereload.js"

Currently, as the error above it tries to load http://localhost:8787/livereload.js but it should be http://localhost:8787/p/56a946ed/livereload.js

We may need to configure hugo server to know this ... 🤔

@yihui
Copy link
Member

yihui commented Oct 9, 2020

I have this trick for RStudio Server:

blogdown/R/utils.R

Lines 656 to 663 in ba977a9

tweak_hugo_env = function() {
if (!is_rstudio_server()) return()
Sys.setenv(HUGO_RELATIVEURLS = 'true')
do.call(
on.exit, list(substitute(Sys.unsetenv('HUGO_RELATIVEURLS')), add = TRUE),
envir = parent.frame()
)
}

but it can't fix the problem of the path of livereload.js, because it's not controlled by the configuration relativeurls. I have also tried to use rstudioapi::translateLocalUrl() to change the baseURL temporarily before, but I don't remember why it didn't work now. I'll experiment one more time.

@cderv
Copy link
Collaborator Author

cderv commented Oct 9, 2020

I have also tried to use rstudioapi::translateLocalUrl() to change the baseURL temporarily before

Yep I am trying that but with no sucess for now.

@yihui
Copy link
Member

yihui commented Oct 9, 2020

This is probably one of the reasons why I didn't use hugo server as the default server. The server started with servr doesn't suffer from this issue, because I have control over the JavaScript.

@cderv
Copy link
Collaborator Author

cderv commented Oct 9, 2020

Yes the live reloading would work ok in that case.

Having RStudio using a location.pathname as proxy to a another port on localhost won't play well with livereload of hugo it seems. Even if we manage to make the file livereload.js to be found, I am not sure the logic for the reloading will work ok in this case. but that is a guess.

It is too bad the relative url trick does not work for this js resource but does for other css and jss of the website. Because the preview is working well.

I don't know if many people are using blogdown on rstudio server (or RStudio cloud)

@yihui
Copy link
Member

yihui commented Oct 9, 2020

We could file a bug report to Hugo, but I don't feel hopeful that it could be fixed (especially if we don't actually submit a PR to fix it), because this is indeed a rare use case.

I don't know if many people are using blogdown on rstudio server (or RStudio cloud)

#124 has some examples. That was the motivation behind the trick of setting relativeurls = true on RStudio Server.

@cderv
Copy link
Collaborator Author

cderv commented Oct 9, 2020

yes it is not really cool this does not work well on Rstudio server. But I guess hugo itself is not working well.

BTW, changing the base url does not seems to affect where the livereload.js is fetched. The requested url in the Browser Network pane still show a request URL without any change to the base url.
No pathname even if I added one.

I think this is related to gohugoio/hugo#1187

@yihui
Copy link
Member

yihui commented Oct 9, 2020

Actually I've been thinking for long about the possibility of connecting to the websocket from R. If we have access to the data that Hugo and the web browser send to each other, we could probably do something (like refreshing a page or auto-navigation) on our end. But I guess in this case, the websocket is not even successfully established in the first place... Another crazy idea is that we could open a browser in a background thread in R to process the request http://localhost:8787/p/56a946ed/livereload.js.

@cderv
Copy link
Collaborator Author

cderv commented Oct 9, 2020

I believe the issue may come from here
https://github.com/gohugoio/hugo/blob/49972d07925604fea45afe1ace7b5dcc6efc30bf/transform/livereloadinject/livereloadinject.go#L62
Where they inject into the HTML files a script tag using in src attributes "/livereload.js" and this does not include the /p/56a946ed path
However it is a relative url right ? Should it work or not as it is ?

@cderv
Copy link
Collaborator Author

cderv commented Oct 9, 2020

The idea I want to experiment is to see if we don't serve the website from memory but we write to a folder, we would have access to the html file and we could change the script tag maybe to correctly load the js script.

I am experimenting with this now.

@yihui
Copy link
Member

yihui commented Oct 9, 2020

Where they inject into the HTML files a script tag using in src attributes "/livereload.js" and this does not include the /p/56a946ed path
However it is a relative url right ? Should it work or not as it is ?

That's an absolute URL relative to the domain root. It's not a completely relative URL. See the second bullet in the last list: https://bookdown.org/yihui/blogdown/html.html This /livereload.js needs to be made truly relative. It shouldn't start with /, but either no / in the beginning, or a path like ../../ in the beginning. In other words, the relative URL should be relative to the current page, not the website root.

Yet another idea is to modify

<script src="/livereload.js?port=4321&amp;mindelay=10&amp;v=2" data-no-instant defer></script>

to

<script src="/p/56a946ed/livereload.js?port=4321&amp;mindelay=10&amp;v=2" data-no-instant defer></script>

on the page using JavaScript (document.getElementsByTagName('script')[0].src = ...). But I'm afraid we can't get this to work automatically, and users will have to add the JS code in their template.

The idea I want to experiment is to see if we don't serve the website from memory but we write to a folder, we would have access to the html file and we could change the script tag maybe to correctly load the js script.

I guess this won't work, because Hugo dynamically injects the <script> when serving the HTML files. It is probably not included in the static HTML files.

@cderv
Copy link
Collaborator Author

cderv commented Oct 9, 2020

I guess this won't work, because Hugo dynamically injects the <script> when serving the HTML files. It is probably not included in the static HTML files.

This is included in the html file written to disk by Hugo server if you set the option -d or --destination. I tried with this on command line

HUGO_RELATIVEURLS=true hugo server -w -d preview

When you use this, hugo is no more serving from memory but from disk

cderv@DESKTOP-D24Q6VQ:~/test-blogdown$ HUGO_RELATIVEURLS=true hugo server -w -d preview
Start building sites … 

                   | EN  
-------------------+-----
  Pages            | 20  
  Paginator pages  |  0  
  Non-page files   |  0  
  Static files     | 15  
  Processed images |  0  
  Aliases          |  0  
  Sitemaps         |  1  
  Cleaned          |  0  

Built in 9 ms
Watching for changes in /home/cderv/test-blogdown/{content,static,themes}
Watching for config changes in /home/cderv/test-blogdown/config.toml
Environment: "development"
Serving pages from /home/cderv/test-blogdown/preview
Running in Fast Render Mode. For full rebuilds on change: hugo server --disableFastRender
Web Server is available at //localhost:1313/ (bind address 127.0.0.1)
Press Ctrl+C to stop

this will create a preview directory (that I believe you could delete when killing the server as when served from memory it even does not exist) - It is just a temp dir on disk instead on cached memory.

In all HTML files there is

<script src="/livereload.js?port=1313&amp;mindelay=10&amp;v=2" data-no-instant defer></script>

inserted in <head>

removing the leading / fix the error 404, the file is found (so 200)

But i now get another error

net::ERR_INVALID_CHUNKED_ENCODING 200 (OK)

@yihui
Copy link
Member

yihui commented Oct 9, 2020

This is included in the html file written to disk by Hugo server

Okay, great! Then there is still hope :)

net::ERR_INVALID_CHUNKED_ENCODING 200 (OK)

Unfortunately I don't know this means...

@cderv
Copy link
Collaborator Author

cderv commented Oct 9, 2020

net::ERR_INVALID_CHUNKED_ENCODING 200 (OK)

Seems related to proxy (again) : https://stackoverflow.com/a/49731895/3436535

@cderv
Copy link
Collaborator Author

cderv commented Oct 9, 2020

And indeed RStudio server get the field in the response header

HTTP/1.1 200 OK
Content-Type: application/javascript
Date: Fri, 09 Oct 2020 14:38:09 GMT
Connection: close
Transfer-Encoding: chunked
X-Content-Type-Options: nosniff
Server: RStudio

@yihui
Copy link
Member

yihui commented Oct 9, 2020

Too bad. Another hope is to see if the rstudioapi package could give us a method to refresh the viewer, e.g., rstudioapi::viewer(refresh = TRUE). Or equivalently, give us the current URL of the page in the Viewer, and we can call rstudioapi::viewer(currentURL) to refresh it. You can ask Kevin Ushey.

Once we have that, we can refresh the viewer by ourselves after compiling the Rmd. Of course, we won't have the auto-navigation, but that shouldn't be too bad, because I think in most cases, users only work on one post, so refreshing the same page should be good enough.

@cderv
Copy link
Collaborator Author

cderv commented Oct 9, 2020

Too bad.

Yep - we could also see with product team if something can be done in RStudio Server to make hugo livereload worked...

Another hope is to see if the rstudioapi package could give us a method to refresh the viewer, e.g., rstudioapi::viewer(refresh = TRUE). Or equivalently, give us the current URL of the page in the Viewer, and we can call rstudioapi::viewer(currentURL) to refresh it. You can ask Kevin Ushey.

Yes it seems a good path to follow - more probability of success I guess.

Of course, we won't have the auto-navigation

If we know the modified file (the one saved or knited), we could call rstudio::viewer on the url of this post, couldn't we ?

Regarding the absolute path insertion, I wonder if livereload should inserted with a relative url to solve the issue linked above on Hugo repo. Seems odd that it is absolute but it seems to have always been that - it was using location.host and location.port before

@cderv
Copy link
Collaborator Author

cderv commented Oct 9, 2020

Regarding the API, there is already a bunch of command available using rstudioapi::exectureCommand

rstudioapi::executeCommand("viewerRefresh") is available... but it refresh to the homepage and does not stays in the current opened page

Full list of commands https://docs.rstudio.com/ide/server-pro/rstudio-ide-commands.html

@yihui
Copy link
Member

yihui commented Oct 9, 2020

Right. I need to refresh the currently opened page. Clicking the refresh button in the Viewer won't work.

@cderv
Copy link
Collaborator Author

cderv commented Oct 9, 2020

So I guess the conclusion of this investigation is that in order to have auto reload on rstudio server,

  • the easiest way is t’oser how to improve rstudioapi to recreate a live reload inside rstudio
  • try to investigate more why upstream live reload does not work and fix also upstream hugo insertion of livereload. Seems less probable and time consuming.

First option seems the one we could try.

@yihui
Copy link
Member

yihui commented Oct 9, 2020

Yes

Actually if rstudioapi::executeCommand('viewerBack') works like the context menu's Back, we could probably do viewerRefresh then viewerBack. But currently it doesn't seem to work that way and I don't know what viewerBack actually does.

image

Of course, if it's possible to do a real reload of the current URL in the Viewer, we won't need this refresh + back hack.

@cderv
Copy link
Collaborator Author

cderv commented Oct 9, 2020

If we can easily know the url of webpage for a post, we could also call rstudioapi::viewer("urltopost") after one source has been modified and saved, or explicitly knitted.

  • Homepage : rstudioapi::viewer("http://localhost:4321")
  • Lorem Ipsum post: rstudioapi::viewer("http://localhost:4321/2015/01/01/lorem-ipsum/")
  • Hello Rmarkdown: rstudioapi::viewer("http://localhost:4321/2015/07/23/hello-r-markdown/")

but rstudioapi::viewer(<currenturl>) would be nice.

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

2 participants