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

[Bug]: live reload rebuild twice when using tailwind watch #714

Closed
1 of 12 tasks
raymclee opened this issue Nov 27, 2021 · 27 comments
Closed
1 of 12 tasks

[Bug]: live reload rebuild twice when using tailwind watch #714

raymclee opened this issue Nov 27, 2021 · 27 comments
Labels
bug Something isn't working

Comments

@raymclee
Copy link

Which Remix packages are impacted?

  • remix (Remix core)
  • create-remix
  • @remix-run/architect
  • @remix-run/cloudflare-workers
  • @remix-run/dev
  • @remix-run/express
  • @remix-run/netlify
  • @remix-run/node
  • @remix-run/react
  • @remix-run/serve
  • @remix-run/server-runtime
  • @remix-run/vercel

What version of Remix are you using?

1.0.6

Steps to Reproduce

Follow the styling section for tailwindcss in remix doc

Expected Behavior

live reload ignore the generated css file change

Actual Behavior

[1] 💿 File changed: app/styles/app.css
[1] 💿 Rebuilding...
[1] 💿 Rebuilt in 226ms
[1] GET /form 200 - - 31.452 ms
[1] 💿 File changed: app/styles/app.css
[1] 💿 Rebuilding...
[1] 💿 Rebuilt in 170ms

@raymclee raymclee added the bug Something isn't working label Nov 27, 2021
@raymclee
Copy link
Author

Im using jit mode

@edgesoft
Copy link
Contributor

edgesoft commented Nov 27, 2021

@raymclee This is not Remix specific. You can come around this using RSync

Take a look at this setup. I used it for my project to eliminate reload twice.
https://github.com/kentcdodds/kentcdodds.com/blob/main/other/pm2.config.js

@edgesoft
Copy link
Contributor

@raymclee I use tailwind like this

https://github.com/edgesoft/moaclayco

@raymclee
Copy link
Author

@edgesoft thanks! It works

@ryanflorence
Copy link
Member

Yeah, at the moment that's just how it is. I'm thinking we should debounce the rebuilds by like 100ms in our watcher and we'd be good.

@kentcdodds
Copy link
Member

debounce the rebuilds

Problem with this is it depends on how fast your machine is. May work $5k MacBooks at one debounce interval (that may even be too long), but for someone on an older/slower machine they may need more time.

I think we should explore a different approach. Having Tailwind support built-in could be one way :) Or support for customizing postcss a bit. That's one "compiler" that I don't expect we'll change in the future so it's probably safer to expose an experimental API for customizing.

@rkusa
Copy link

rkusa commented Nov 29, 2021

Instead of adding a workaround to Remix, it could also be an option to try to get postcss to not write files if their content didn't change (there is also existing issue for that postcss/postcss-cli#320).

@kentcdodds
Copy link
Member

not write files if their content didn't change

This is the real solution 👍

@kentcdodds
Copy link
Member

That issue isn't quite what we need. That issue just says: "if the input is the same as the output then don't save the file" but what we need is "if the output is the same as what's currently in the output then don't save the file." In any case, I'm going to look into implementing what we need.

@kentcdodds
Copy link
Member

There we go :) postcss/postcss-cli#417

@raymclee
Copy link
Author

Is everyone using postcss-cli? I just tried to use tailwindcss instead of postcss-cli and it does cause rebuilding twice sometime but only reload once.
can anyone confirm with it?

@raymclee
Copy link
Author

raymclee commented Dec 1, 2021

Is everyone using postcss-cli? I just tried to use tailwindcss instead of postcss-cli and it does cause rebuilding twice sometime but only reload once. can anyone confirm with it?

ok still reload twice XD

@rkusa
Copy link

rkusa commented Dec 4, 2021

I am optimistic that the suggested change to postcss-cli lands eventually so if you are like me and want something now, but something that is very easy to migrate once the postcss-cli PR has landed, you can create a wrapper around the postcss-cli that 🙈-patches fs.outputFile:

In my case bin/postcss.mjs:

#!/usr/bin/env node

import fs from "fs-extra";
import "postcss-cli/index.js";

const PREVIOUS_CONTENT = new Map();

const _outputFile = fs.outputFile;
fs.outputFile = async function outputFile(path, data) {
  const prev = PREVIOUS_CONTENT.get(path);
  if (prev && prev === data) {
    return; // unchanged
  }

  PREVIOUS_CONTENT.set(path, data);

  return _outputFile(path, data);
};

In my package.json script section:

- "build:css": "postcss --config ./config/ --dir app/styles styles/global.css",
+ "build:css": "./bin/postcss.mjs --config ./config/ --dir app/styles styles/global.css",

Once postcss CLI got the change, I am just going to remove the file and revert the build:css change.

Not pretty, but works at least for me for the time being.

@kentcdodds
Copy link
Member

That workaround works for now, but for anyone coming to this issue later it's very temporary, and we'll have a better solution for this soon! Thanks @rkusa :)

@kentcdodds
Copy link
Member

My PR was merged into postcss-cli!

If you update your version to 9.1.0 or greater you should no longer get a double-refresh when you save a file unless you actually do change the CSS (like in the Tailwind JIT case, if you change the class name on an element). In that case you'll still get the double-reload. @ryanflorence had some ideas of how to avoid that once and for all. But the nice this is with postcss-cli 9.1.0 you'll no longer get an unnecessary rebuild :)

@kentcdodds
Copy link
Member

Just upgraded my app: kentcdodds/kentcdodds.com@3f6637c

I was able to remove the rsync workaround thing.

Here's how things worked without the rsync workaround before postcss-cli 9.1.0:

image

And here's how things work now (still without the rsync workaround) with postcss-cli 9.1.0:

image

Much better :)

@kentcdodds
Copy link
Member

If anyone wants to dig into the tailwind-cli to see whether this sort of change is necessary/would help that would be welcome :)

@babycourageous
Copy link

Thanks for this awesome improvement @kentcdodds !

@g3offrey
Copy link

@kentcdodds I just tried with Tailwind cli and it seems to rebuild twice even if there is no CSS changes.
Remix says that the CSS output file of Tailwind changed even if the content is the same ☹️

@raymclee
Copy link
Author

@kentcdodds I just tried with Tailwind cli and it seems to rebuild twice even if there is no CSS changes. Remix says that the CSS output file of Tailwind changed even if the content is the same ☹️

same here

@g3offrey
Copy link

g3offrey commented Dec 15, 2021

I found a solution on Tailwind side. The same way that you found one for PostCSS. I opened a PR here, if you want to check it out 😄
tailwindlabs/tailwindcss#6550

Also I created a reproduction repository

@g3offrey
Copy link

g3offrey commented Jan 4, 2022

It should now be fixed for Tailwind CLI. I'll try again in the next 24 hours (when I can access my laptop). And if it works, I guess we could close the issue.

@g3offrey
Copy link

g3offrey commented Jan 5, 2022

So I just tested it and it works fine now for the latest version of Tailwind CLI.
When you update content in TSX the Remix server restart once.
This is great 👍

However if you add or remove a Tailwind class (so the CSS file changes) there is still a double rebuild. I think this is expected behavior at the moment.

Do you think we can close this ticket @kentcdodds ? 😊

@girishk21
Copy link

would devServerBroadcastDelay in remix.config help to avoid that? 🤔

@g3offrey
Copy link

g3offrey commented Jan 5, 2022

@girishk21 I wasn't aware of this configuration variable.
However as I understand it, it won't avoid to double build the project. It will only avoid to double refresh the browser.
It tells to wait X seconds before refreshing the browser.

@kentcdodds
Copy link
Member

Yeah, we have one more thing to improve this a bit more and it's basically: if the browser gets a notice that a second build has started, wait to refresh even if the first build has finished. Let's leave this open for that.

@chaance
Copy link
Collaborator

chaance commented Apr 19, 2022

Closing for now, as this is an inevitable issue if you're running multiple processes. devServerBroadcastDelay should help, and we'll have more to say on DX improvements here in the not-too-distant future.

@chaance chaance closed this as completed Apr 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

9 participants