Slow page reload (5s) if the main editor does not show the main file

[DennisKehrig]

5s is the static server extension's default timeout when it waits for Brackets to handle a "request" event. The HTML instrumentation is apparently only responding when the HTML file is the main file open in Brackets. The code responsible is in LiveDevelopment.js - the request handler is registered in _openDocument and unregistered in _closeDocument. These methods are called when switching to or from a live dev file.

Ideally, the page continues to work even after the live development session has been closed again (it may still be open in the browser). However, then HTML instrumentation is no longer needed, so the node server could just serve the files directly. For that, we need to undo the call to _serverProvider.setRequestFilterPaths in _openDocument. There does not seem to be an API for that (i.e. removeRequestFilterPaths).

While live development is still active, a reload should always trigger HTML instrumentation, I think. Maybe it is only needed when the HTML file is the active file - but if a reload when saving an included LESS file disables HTML instrumentation, another reload to enable it is required when switching back to the HTML file.

[DennisKehrig]

Regarding my comment about the missing API: apparently now setRequestFilterPaths acts more consistently with its name, it used to add new paths to the old ones. Now they are replaced, such that setRequestFilterPaths([]) would be the undo operation.

[njx]

Reviewed, medium priority to @jasonsanjose for this sprint. I think @DennisKehrig is right--we should instrument HTML even if the HTML file isn't current. I ran into a similar issue when I was doing my prototyping of JS file instrumentation for live development. I fixed it by making it so that instead of specifying just the current document as a filter path, we handle all requests for any item in the current project, and then if the request is for a file that needs to be instrumented, we create the appropriate document on the fly. I don't know if that would work for HTMLDocument exactly without some other work--it's a pretty major change--but I think ultimately it's a better way to go than having the constraint of only having the current file be open as a live document for instrumentation.

[jasonsanjose]

I think there's maybe 2 different problems here with possible different solutions. Let me see if I understand right:

From @DennisKehrig

1. Open Getting Started/index.html
2. Start live preview
3. Open main.css
4. Hit the refresh button in Chrome
   Expected: Fast refresh
   Actual: Slow, 5s refresh due to the fact that StaticServer is still filtering for index.html but LiveDevelopment isn't sending instrumented content because index.html is no longer the active live document

From @njx
Request: Pass all HTTP requests for any file through live development and add instrumentation when possible.

For @DennisKehrig's original problem, we could simply correctly update the filter so that the refresh request reverts back to static file serving. @njx is suggesting something different, more useful, but larger in scope.

It would be a major change to Live Preview to separate instrumentation from editing (e.g. tracking cursor position for HTML highlighting and refreshing the browser on save). Do we want to tackle that now or as a separate user story? I think something of that scope would fall under the live development refactoring stories that we talked about at an architecture meeting back in April.

I can try to look at some low impact changes to make today, knowing that we're looking at a larger refactoring in the near future. I don't think I'd spend much more than a day working on it though unless I run out of sprint 25 tasks. Thoughts?

[DennisKehrig]

Hey @jasonsanjose, I think we should go for the easy fix now (this is basically a regression, apparently the code for the HTML instrumentation was previously elsewhere) and track the larger story separately.

[redmunds]

(moved comment to pull request)

[njx]

Ah, that makes sense--we don't actually need to reinstrument the HTML right now when it's not the current document since we're only using it for highlighting (in my case, I needed to reinstrument the JS all the time). I do think we'll eventually need to generalize the architecture here though.

[DennisKehrig]

But then you have to reload the page when the user switches back to the HTML document (assuming it has been reloaded while in a different file before)

[njx]

Also true. As a stopgap, we could detect that case and auto-reload when switching back to the HTML file. But that would be another reason to make it so files other than the current file could be instrumented.

I actually don't think that's a huge amount of work, but it's probably more than we could fit into this sprint. So it does seem like we need to take care of reloading the page when you switch back to its HTML file--otherwise highlighting will be broken when you switch back to it, right?

[DennisKehrig]

Yes, that is what I would expect.

BTW, do you happen to know what the intent was behind the refactoring? A while ago, the HTML instrumentation was on the whole time while live preview was enabled and reloads were snappy.

[redmunds]

FBNC to @DennisKehrig

[DennisKehrig]

The reload is fast now, but as expected highlighting doesn't work anymore after reloading the page when the HTML file isn't the current file in Brackets. As a workaround, the user can reload the page manually after going back to the HTML file, but this should be done automatically. Back to @jasonsanjose, I assume?

[jasonsanjose]

Closing, see #3792 for the additional bug