Should Browsersync watch the build folder too?

Created on 27 December 2024, 2 months ago

Making changes to _variables.scss, I noticed that my Browsersync instance of the site wasn't updating, and I wondered if it had stopped working.

I looked at webpack.mix.js and saw that it watches css and twig files in components and assets. Those would pick up any changes to styling in components, but not to the variables that affect theme colour etc.

Is there a reason why I shouldn't do this?

--- a/web/themes/custom/subtheme/webpack.mix.js
+++ b/web/themes/custom/subtheme/webpack.mix.js
@@ -40,6 +40,8 @@ mix.browserSync({
     'components/**/*.js',
     'components/**/*.twig',
     'templates/**/*.twig',
+    'build/css/*.css',
+    'build/js/*.js',
   ],
   stream: true,
 });

It fixes my problem, but I'm wondering if this is a "bad idea ™"

Is there a reason why Radix doesn't do this out of the box?

💬 Support request
Status

Active

Version

6.0

Component

Code

Created by

🇬🇧United Kingdom JamesOakley Kent, UK

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @JamesOakley
  • 🇺🇸United States danchadwick Boston

    Not my area of expertise. Can anyone else chime in, ideally with an exact patch?

  • 🇬🇧United Kingdom JamesOakley Kent, UK

    Whether doing this is wise or sensible, I cannot say - it's why I asked the question.

    But I can help with the "exact patch" request.

  • Hey guys, I'd say that could possibly make sense in some scenarios but not generally no. but there might be some downsides:
    - Watching build files could cause recursive reloading
    - Build files are outputs, not sources
    - Changes to build files are already handled by laravel-mix

    in general we can do a bit of testing and see how it turns out to be but BrowserSync should only watch source files

  • 🇬🇧United Kingdom JamesOakley Kent, UK

    @doxigo, that's fair enough.

    But, strictly speaking, components/**/*.css aren't source files. Shouldn't BrowserSync be watching components/**/*.scss instead?

    Which leads to my use-case, and why I wanted to add the build directory: Shouldn't BrowserSync be watching src/scss/base/**/*.scss as well? In my case, I was changing Bootstrap variables in base/init/_variables.scss, to fine-tune colour and layout, and having to refresh the page each time.

  • @jamesoakley Yes to all. any SCSS/non-build JS files (the ones that start with a _) should be watched. could you do a MR then please?

  • 🇬🇧United Kingdom JamesOakley Kent, UK

    So, I'm working on this and I've found a potential problem. If you watch the source files, rather than the generated ones, the browser resync is triggered when you save an updated source file (obviously). That also triggers sass to regenerate the css files.

    If I amend a .scss file in components, it mostly works.

    If I amend base/_variables.scss, that's a more foundational file to alter, so requires more css files to be regenerated, which means it takes (in my case) a second or two for sass to do its magic. My browser reloads the BrowserSync page much faster than that. So the page reloads using the unchanged CSS, and I have to hit F5 when the compilation has finished running. Hitting F5 manually defeats the purpose of using BrowserSync.

    What this is actually exposing is that there is a race condition with monitoring any source files for changes. It happens that I've now started watching one that will trigger a more time-consuming CSS rebuild, which exposes the race condition. The fact that I get away with it when altering a css file in the components folder - is a fluke.

    Happy to keep working on a MR for this, but I'd need a steer as to how it should be solved. I still need a way to update _variables.scss and trigger BrowserSync. So far, watching the built css files is the only way I can make this work.

  • Hey James, thanks for a thorough testing, in the end if watching the build does the job, go for it. In my head, we needed to watch the source rather than the build but if it fails as you suggest, well... it fails.

    There's no harm in switching the browserSync watch in general.

  • 🇺🇸United States danchadwick Boston

    Does it build twice, once when the sources is changed and again when it detects the build file changed?

    Also wondering if there is a time threshold that mix supports, like don't trigger the browser reload until the job is done.

  • 🇬🇧United Kingdom JamesOakley Kent, UK

    Hi Dan,

    If I turn off watching the build directory, but watch the sources, then change _base/variables.scss, then the browser reloads when that file is saved, but does not reload again after the build job has completed.

    As you say, the ideal would be if the reload was triggered by a source file changing, but waited until mix was finished and idle. I don't know if there is any option for that.

  • 🇺🇸United States danchadwick Boston

    Presumptuously you have to watch the sources (or you won't start the build) and watch the destinations (so you can re-trigger the browser refresh). What I'm wondering is if you do that, do you get two builds -- source change -> build -> destination change -> build -> destination "changed" to the same file contents.

    I'm surprised this doesn't make an infinite loop.

    Ideally you'd like to decouple the build (based on source) and the refresh (based on destination). Possibly?

  • 🇬🇧United Kingdom JamesOakley Kent, UK

    If you watch the source and the destination, the browser refreshes twice - the first time when you save the source file, the second time when the build process has finished. You do not get an endless loop.

    As I understand it, the reason is that the watch process is doing two things.

    1. It's watching for source files to change so that it can regenerate the final css, js and resource files.

    2. It launches an instance of BrowserSync, which itself needs to be told what files to watch to see when a reload is required.

    The first of those is kicked off by this:

    mix.sass("src/scss/main.style.scss", "build/css/main.style.css");
    
    for (const sourcePath of glob.sync("components/**/*.scss")) {
      const destinationPath = sourcePath.replace(/\.scss$/, ".css");
      mix.sass(sourcePath, destinationPath);
    }
    

    main.style.scss links to _init.scss, which in turn links to the various files (like _variables.scss) in the base directory. When any of those files are changed, the css gets rebuilt.

    The second of those (BrowserSync) is kicked off by this statement:

    mix.browserSync({
      proxy: process.env.DRUPAL_BASE_URL,
      files: [
        'components/**/*.scss',
        'components/**/*.js',
        'components/**/*.twig',
        'templates/**/*.twig',
      ],
      stream: true,
    });
    

    That tells it what files BrowserSync will update on.

    You would get an endless loop if the first of those steps (watching source files to know when to regenerate the css) watched for the generated files to change.

    I'm proposing making it so that the second of those steps (watching files to know when to reload BrowserSync's proxy site) watches the compiled files instead of the source files.

    That way, the flow becomes:

    • Save a source file
    • This triggers the CSS to regenerate
    • The regenerated CSS in turn triggers BrowserSync to refresh its styling

    This is an improvement on the current flow, which is:

    • Save a source file
    • This triggers:
      • the CSS to regenerate
      • BrowserSync to refresh its styling
    • If you're lucky, the CSS has finished regenerating before the browser finishes refreshing.
  • 🇺🇸United States danchadwick Boston

    @jamesoakley - Open a MR with your suggested change? I don't use browsersync so I can't really test that part.

  • Pipeline finished with Success
    about 2 months ago
    Total: 136s
    #395957
  • 🇺🇸United States danchadwick Boston

    Thx. Merged.

  • 🇬🇧United Kingdom JamesOakley Kent, UK

    Apologies for the second merge-request. Working with this updated mix config file, I've discovered that it was a mistake to remove ./components/**/*.css because those files are not incorporated into the files generated for the ./build directory. So this latest MR puts that back into webpack.mix.js.

  • Pipeline finished with Success
    about 1 month ago
    Total: 140s
    #399566
  • 🇺🇸United States danchadwick Boston

    Forgive my inexperience with GitLab, but I think this needs rebasing on the current 6.0.x head. I'm scared that if I click "Merge", it will revert the changes where the issue fork is behind radix.

  • 🇬🇧United Kingdom JamesOakley Kent, UK

    I think this needs rebasing on the current 6.0.x head.

    That should now be rebased and good to merge

  • Pipeline finished with Skipped
    about 1 month ago
    #405314
  • Pipeline finished with Success
    about 1 month ago
    Total: 228s
    #405312
  • Automatically closed - issue fixed for 2 weeks with no activity.

Production build 0.71.5 2024