adamchainz / django-browser-reload

Automatically reload your browser in development.
MIT License
512 stars 26 forks source link

Implement a check that the development server is back before reloading a page #46

Closed timonweb closed 2 years ago

timonweb commented 2 years ago

Description

Hi Adam,

I'm trying to get django-browser-reload work with Tailwind CSS, and I'm almost there. However, I noticed a weird lag happening sporadically after I edit HTML code - the page reloads and shows the "No server response" and then reloads again and shows the working page. I guess that's because the reloader tries to fetch a page while the Django server is restarting. Please see the attached video. The behavior I'm describing happened in the second part.

2022-01-09_23-24-07 (1)

So I was wondering if it's possible to implement a check that the development server is back before reloading a page?

PS: Thanks for the package!

adamchainz commented 2 years ago

There is already such a check. The JS detects the server restarting and waits until it manages to connect to the 'events' view before it triggers a reload.

I'm not sure what's going on in your example. Do yyou have any other scripts on the page, a proxy server, etc.?

adamchainz commented 2 years ago

Oh I had an idea what may be happening:

  1. You save the template change
  2. In parallel:
    • Django's autoreloader detects this change, and django-browser-reload sends a browser message to reload
    • Tailwind detects the template change, and begins rebuilding the CSS
  3. The CSS change causes runserver to restart
  4. The browser reacts to the message and reloads, but the server is unavailable as it's restarting

Not sure why you then get a second reload and actually see the content. The JS should stop running at this point.

Anyway, do you think this could generally be what's happening?

One question I have is: why does django even need to restart when the generated CSS changes? Reloading Django can get seriously slow with a good amount of dependencies, as Python has to reprocess all the imports. If you could rearchitect your library to avoid the restart, that would be ideal.

On my project using plain tailwind, there wouldn't be a restart. The tailwind watch process just places CSS into the static folder, and that doesn't cause Django to restart.

adamchainz commented 2 years ago

I had a second theory - perhaps the development server isn't actually reloading, and what's happening is something like:

  1. You save the template change
  2. In parallel:
    • Django's autoreloader detects this change, and django-browser-reload sends a browser message to reload
    • Tailwind detects the template change, and begins rebuilding the CSS
  3. The browser starts reloading
  4. Tailwind saves the CSS, which Django's autoreloader sends, and django-browser-reload sends another message
  5. The browser triggers window.location.reload() whilst the page was already unloading, leading to the current reload being cancelled, and the flash of the "could not connect" screen.

I think we could solve that kind of problem by debouncing via threading.Timer(). Are you using Watchman? It already debounces file change events, with a small window.

timonweb commented 2 years ago

Adam, thank you for taking your time and looking into the issue.

What you've described is what's actually happening.

My library uses barebones Tailwind without any custom node reloaders. I got rid of them in favor of your package. So nothing is sitting in between. It's just the tailwind process running and the stylesheet link included on the page.

I narrowed down the issue to the StatReloader reloading the dev server on template changes. Here I found an issue reporting this regression in Django: https://code.djangoproject.com/ticket/32744, and it's marked as fixed, but they seem to have fixed not what had been reported.

The weirdest part is that the observed behaviour isn't happening all the time. Not sure how StatReloader works, but sometimes it treats a template change (for example, addition or removal of a CSS class) as a Python module change and restarts the server. Not sure what causes that. Maybe you have some insight.

Anyways, I'll continue digging when I get some free time and will report back.

Thanks.

timonweb commented 2 years ago

I had a second theory - perhaps the development server isn't actually reloading, and what's happening is something like:

The server is reloading, I see that in the terminal.

Are you using Watchman?

No I don't use Watchman, just standard StatReloader.

adamchainz commented 2 years ago

Not sure how StatReloader works, but sometimes it treats a template change (for example, addition or removal of a CSS class) as a Python module change and restarts the server. Not sure what causes that. Maybe you have some insight.

If it's only sometimes, that is weird. It should be deterministic.

Inside django.utils.autoreload, there are a bunch of debug log messages. If you change your log settings to show those messages, they may help find out why it's triggering the reload.

timonweb commented 2 years ago

Adam, I've enabled logging and here's what I get when the server restarts:

[10/Jan/2022 10:25:50] "GET / HTTP/1.1" 200 762
[10/Jan/2022 10:25:51] "GET / HTTP/1.1" 200 761
~/www/django-tailwind/example/theme/static/css/dist/styles.css changed, reloading.
~/www/django-tailwind/example/theme/static/css/dist/styles.css changed, reloading.
Watching for file changes with StatReloader
Watching for file changes with StatReloader
Performing system checks...

System check identified no issues (0 silenced).
January 10, 2022 - 10:25:58
Django version 4.0.1, using settings 'project.settings'
Starting development server at http://127.0.0.1:8000/
Quit the server with CONTROL-C.
[10/Jan/2022 10:25:59] "GET / HTTP/1.1" 200 762
[10/Jan/2022 10:25:59] "GET /static/css/dist/styles.css HTTP/1.1" 200 15122

The key is this message:

~/www/django-tailwind/example/theme/static/css/dist/styles.css changed, reloading.

It only happens when I add a new CSS class to the HTML, which triggers Tailwind to rebuild the stylesheet. But should the autoreloader restart the server when a static asset changes? Is it expected behaviour? I assume it's not.

adamchainz commented 2 years ago

That is indeed not expected behaviour! I made a mistake in the static file reload code - fixing now.

adamchainz commented 2 years ago

Released in 1.1.1, can you try that out?

timonweb commented 2 years ago

great, thank you, Adam. I'll be back in 2-3 hours and will try it out.

timonweb commented 2 years ago

Adam, I upgraded and it works flawlessly now, thank you for such a quick reaction!

ahmedaljawahiry commented 2 years ago

Was just about to raise this exact issue - updating to the latest version fixed it. Thanks both.