Improve editing experience when using `--livereload` by squidfunk · Pull Request #3391 · mkdocs/mkdocs · GitHub
Skip to content

Improve editing experience when using --livereload#3391

Merged
oprypin merged 4 commits into
masterfrom
fix/livereload-editing-ux
Oct 14, 2023
Merged

Improve editing experience when using --livereload#3391
oprypin merged 4 commits into
masterfrom
fix/livereload-editing-ux

Conversation

@squidfunk

@squidfunk squidfunk commented Sep 13, 2023

Copy link
Copy Markdown
Contributor

Closes #3389 and supersedes/closes #3390.

I've refactored the livereload functionality. It's more predictable now and solves another problem: it's currently not possible to open more than 6 tabs, because browsers are limited to 10 connections per destination, and the livereload connections queue up and block further connections. The problem is that every tab will keep an active livereload connection, regardless of whether it is visible or not.

This makes working on several pages in parallel impossible – see the never ending loading indicators here:

Bildschirm­foto 2023-09-13 um 13 44 58

This PR addresses this issue and makes livereload more efficient.

  1. Logic is refactored into two functions, poll and stop, that are called when certain events happen
  2. The livereload server connection is cancelled when the tab becomes inactive
  3. Once a tab becomes active, the livereload connection is immediately established again

The limitation on the number of open pages is gone:

Bildschirm­foto 2023-09-13 um 13 51 17

If you switch back and forth between two tabs, you can inspect the network and see the new behavior:

Bildschirm­foto 2023-09-13 um 13 53 44

IMHO, there's no upside in keeping livereload connections for inactive tabs alive. This implementation improves the editing experience, as it is sometimes necessary to open more than 5 or 6 pages simultaneously ☺️

@squidfunk squidfunk requested a review from oprypin September 13, 2023 11:54
@ultrabug

Copy link
Copy Markdown
Member

@oprypin

This comment was marked as abuse.

@oprypin

This comment was marked as abuse.

Comment thread mkdocs/livereload/__init__.py Outdated
@oprypin

This comment was marked as abuse.

@squidfunk

Copy link
Copy Markdown
Contributor Author

There is an important difference now, though: tabs will never reload anymore unless they're currently opened. I don't know, maybe some users relied on seeing a visual indicator that something is happening in the browser when they edit a page?

Yes, as I mentioned. The thing is: if you're working on any page, the currently active page will reload. The other pages will only reload once you focus them again, yes, but as long as the current page reloads as long as you're working on it, it's the exact same information as before. I've been working with those changes all day and have to say it is so much better than before. Much faster, and I don't have to restart MkDocs again and again when the server starts hanging. Try working a little with it, I think you'll eventually agree that the new behavior is better. If not, you can turn down this PR, but we then need to fix the hanging livereload server someway else.

Co-authored-by: Oleh Prypin <oleh@pryp.in>
@squidfunk

Copy link
Copy Markdown
Contributor Author

Another question: why does this have to wait for MkDocs 1.6.0? It's essentially a bugfix. Other than that, do you have plans when MkDocs 1.6.0 will be out? If I look at the milestone, it seems that there are currently 9 open issues that should go into 1.6.0, several of which are likely to take longer as they're quite substantial changes, particularly:

Thus, work on 1.6.0 hasn't really started yet, except for cache busting which was a linked PR. Hoever, IMHO, cache busting is something that should not go into MkDocs, as it is very hard to get right if you do it right. We had a long conversation on this topic on Gitter some time ago, where I shared my concerns to implement this into MkDocs.

@oprypin

This comment was marked as abuse.

@oprypin

This comment was marked as abuse.

@squidfunk

Copy link
Copy Markdown
Contributor Author

I don't understand why a small bugfix that improves the developer experience significantly has to wait for months until the rest of the issues was tackled, all of which have probably deeper implications and consequences, but okay then. I'm not saying that the code is free of bugs or that I'm 100% sure that we caught all conditions, but we can fix stuff in subsequent releases. I'm also not saying we should release it now, we should first test it a few days before we're confident that it works.

Releasing bugfixes a little faster wouldn't hurt the project, au contraire. Especially since there's funding now.

@oprypin

This comment was marked as abuse.

@squidfunk

squidfunk commented Sep 13, 2023

Copy link
Copy Markdown
Contributor Author

The "downside" is a change in behavior. Every release is a change in behavior, that's what maintaining and developing software is all about. If users complain about it, we can reconsider if we can somehow replicate the previous behavior or even revert to the previous behavior, but mkdocs serve is very much unusable right now.

@pawamoy

pawamoy commented Sep 14, 2023

Copy link
Copy Markdown
Contributor

I personally cannot find a use-case where I would need inactive tabs to be reloaded. When I work on docs, I usually have a a single docs tab open, and I only need it reloaded when I'm on it. I suppose that 99.5% of MkDocs users use serve like that. Maybe a few users expect inactive tabs to reload as well, triggering some Javascript or something 🤷?

Also, I've seen MkDocs "block" many times after I triggered a reload (saving a file), with a single tab open. Usually I simply Ctrl-C and restart the server. I understand there's also #3390 that fixes this particular issue, but if #3391 (this PR) fixes it as well, and fixes more things, then I'd vote to merge it instead of #3390 for 1.5.3.

Also also, IMO this is not super critical either, so I wouldn't mind waiting a bit longer. It would be sad to have to wait months though. Not sure what's the ETA for 1.6.0. And I understand that heavier users of serve (like Martin) who work with more than 6 tabs open could be less patient that I am.

My 2 cents!

@ultrabug

Copy link
Copy Markdown
Member

Likewise, while I'm satisfied with the actual bug fix in #3390 I must admit that I don't mind going straight for #3391 as well if we all agree about it (and I don't mind waiting for 1.6 on the other hand).

This may be personal and biased but I don't like having websites update their views "in the background" while I'm not looking at them. It's sane and more resource efficient to have the tabs reload themselves only when focused again.

For the users really in need of that feature there are browser extensions out there anyway so it's not like we'll be hard breaking things IMHO.

@ultrabug ultrabug left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From what I can logically read, this looks good and cool 👍

@squidfunk

Copy link
Copy Markdown
Contributor Author

Friendly ping to ask when 1.6.0 will be released. It's now almost been 4 months since this fix was proposed, and 3 months since it has been merged. It would make my life (and possibly the life of others) much easier. If any bugs with this code are encountered, you can happily assign them to me and I'll fix them quickly.

@squidfunk

squidfunk commented Feb 21, 2024

Copy link
Copy Markdown
Contributor Author

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

Successfully merging this pull request may close these issues.

mkdocs serve --livereload hangs, forces to CTRL-C and restart

4 participants