HMR randomly fails in browser console with no obvious cause/pattern

AaronBeaudoin commented 1 year ago

What version of Remix are you using?

2.0.1

Are all your remix dependencies & dev-dependencies using the same version?

[X] Yes

Steps to Reproduce

None. I haven't been able to reliably reproduce this issue because it just happens randomly. Here's what I've been doing:

Link to a CSS file in root.jsx like so:

import styles from "./styles.css";

export const links = () => [
  { rel: "stylesheet", href: styles }
];

Toggle a color back and forth between some values:

.color {
  background-color: hsl(0, 100%, 50%);
}

After a few times the HMR will usually randomly fail with an error like this:

Expected Behavior

HMR either always works or always doesn't depending on whether my code is correct.

Actual Behavior

HMR randomly sometimes doesn't work with no obvious reason.

I'm on MacOS 13.5.2, using Node 18.18.0, and Remix 2.0.1.

fwardcorpaxe commented 1 year ago

I've noticed the same thing in Ubuntu (through Windows WSL2).

It's fairly inconsistent but still seems to be race condition related.

AaronBeaudoin commented 1 year ago

I created this issue in response to a conversation in the Discord server. User @xHomu suggested I try creating this server.js file and see if I can create errors again while running npx remix dev --manual -c "node server.js". To my surprise, I wasn't able to get the errors to come up. I went back and tried regular npx remix dev and I was able to get the errors again.

The way that I got the errors to come up was to create a quick script to toggle the CSS color really fast to try and bog down the server. Here is the simple code, which I ran as node toggler.js with the dev server already running:

import fs from "fs";
let toggle = false;

setInterval(() => {
  fs.writeFileSync("app/color.css", `.color {
    background-color: hsl(${toggle ? 0 : 180}, 100%, 50%);
  }`);
  toggle = !toggle;
}, 200);

This class was applied in a simple app/routes/_index.jsx like this:

export default function App() {
  return (
    <div className="color">
      Hello world!
    </div>
  );
}

And my app/root.jsx looks like this:

import {
  Links,
  LiveReload,
  Meta,
  Outlet,
  Scripts,
} from "@remix-run/react";

import styles from "./color.css";

export const links = () => {
  return [
    {
      rel: "stylesheet",
      href: styles
    }
  ];
};

export default function App() {
  return (
    <html>
      <head>
        <link
          rel="icon"
          href="data:image/x-icon;base64,AA"
        />
        <Meta />
        <Links />
      </head>
      <body>
        <Outlet />
        <Scripts />
        <LiveReload />
      </body>
    </html>
  );
}

xHomu commented 1 year ago

That's odd, @pcattori shouldn't remix app server be the exact same as the Express template server.js?

AaronBeaudoin commented 1 year ago

Is there any way that this code could be making the difference?

AaronBeaudoin commented 1 year ago

Also, just a note that you'll need to change the interval in my toggler.js snippet above in order to "reliably" reproduce this issue. I found that on my Windows machine 200ms was a good rate, but on my Mac I had to lengthen the time to 350ms to get the error to show up most frequently.

AaronBeaudoin commented 1 year ago

Also, I just read that Vite has "been on Remix's radar" since July 28, 2022. That's a year and a half ago. What this seems to indicate is that important DX features like reliable HMR are very low on Remix's priority. Is that true?

EDIT: Woah, just saw this comment was added only 30 minutes ago.

kiliman commented 1 year ago

What this seems to indicate is that important DX features like reliable HMR are very low on Remix's priority.

That's not a very nice take. There's no need for snark.

Remix implemented support for HMR as well as HDR (Hot Data Reload) in v2. @pcattori spent a long time working on it. And yes, they are experimenting with making Vite the default tooling in a future release.

AaronBeaudoin commented 1 year ago

Nice vs. not nice and "snark" wasn't a factor in my question, although I could have worded it better. I directly indicated the impression I got from reading through the Remix discussion on Vite and asked whether it was true.

It sounds like you're implying that Remix has invested significant time in DX. That's great to hear, although you also notably didn't address why Remix has had Vite on the radar for quite a while with no indication of what's next. DX obviously does matter to an extent in the Remix design—although transparency and alignment with web standards appears to be stronger defining goals. There's nothing wrong with any of that.

brophdawg11 commented 1 year ago

I believe this is related to https://github.com/remix-run/remix/issues/7466, specifically this comment

AaronBeaudoin commented 1 year ago

That comment from @pcattori seems to check out. Yet I was experiencing the issue without fail for every single HMR update. (I only put together the script above in an attempt to make it more reproducible.) So under certain conditions it is clearly possible for the issue to occur pretty much without fail.

I suspect that in my case this is the problem:

I wanted to use UnoCSS (it's a Tailwind compatible CSS engine that totally kicks butt), so I installed these dependencies:
```
"devDependencies": {
 "unocss": "^0.56.5",
 "concurrently": "^8.2.1"
}
```
I can't use the Vite integration for UnoCSS because Remix doesn't use Vite, and the PostCSS integration for UnoCSS is currently experimental. So I added scripts like this to simply use the UnoCSS CLI in watch mode, which from my research seems to align with the "Remix" approach for situations like this:
```
"scripts": {
 // 1. Watch files matching app/**/*.{js,jsx,ts,tsx}
 // 2. Output classes into app/uno.css
 "unocss:dev": "unocss -wo 'app/uno.css' 'app/**/*.{js,jsx,ts,tsx}'",
 // 3. Run UnoCSS concurrently with `remix dev`
 "dev": "conc -n R,U 'remix dev' npm:unocss:dev"
}
```

Now I simply include app/uno.css in root.jsx like this (the other import is just the base CSS reset which UnoCSS does not include in the output for flexibility reasons):

import styleReset from "@unocss/reset/tailwind.css";
import styleUno from "./uno.css";

export const links = () => [
 { rel: "stylesheet", href: styleReset },
 { rel: "stylesheet", href: styleUno }
];

Finally, I run npm run dev. Whenever I make an update to CSS classes in my JSX it immediately triggers a rebuild. Since UnoCSS is in watch mode, it also picks up the change and updates app/uno.css, so my theory is that this is triggering a second rebuild so quickly that it's basically guaranteed the browser won't be finished with the first HMR update. I'm not sure how to verify this, though.

akomm commented 1 year ago

Yes, I think https://github.com/remix-run/remix/issues/7466 is the same.

remix-run / remix