Hydration errors when enabling JavaScript

[grantnorwood]

@colbywhite You might have insight on this ...

When enabling JS in root.tsx:

export default function App() {
  // TODO: ❗️ Re-enable javascript before merging ❗️
  // const matches = useMatches();
  // const includeScripts = matches.some((match) => match.handle?.hydrate);

  const includeScripts = true;

  // ...
}

... in NextEventInfo.tsx, the <time /> is different on the server vs client. Is this a simple "wrap it in a useEffect()" fix and set a default for both server + client?

Browser console error:

Warning: Text content did not match. Server: "Thu, Mar 2, 6:00 PM CST" Client: "Thu, Mar 2, 6:00 PM CST"
    at time
    at p
    at NextEventInfo (http://localhost:3000/build/routes/index-4JFQHTNB.js:78:3)
    at div
    at div
    at div
    at Index (http://localhost:3000/build/routes/index-4JFQHTNB.js:231:31)
    at RemixRoute (http://localhost:3000/build/_shared/chunk-6KBQH6TQ.js:5236:3)
    at Outlet (http://localhost:3000/build/_shared/chunk-6KBQH6TQ.js:2490:26)
    at main
    at body
    at html
    at App
    at RemixRoute (http://localhost:3000/build/_shared/chunk-6KBQH6TQ.js:5236:3)
    at Routes2 (http://localhost:3000/build/_shared/chunk-6KBQH6TQ.js:5219:7)
    at Router (http://localhost:3000/build/_shared/chunk-6KBQH6TQ.js:2494:15)
    at RemixCatchBoundary (http://localhost:3000/build/_shared/chunk-6KBQH6TQ.js:3716:1

We don't currently have JavaScript enabled since it's all server rendered. But while I was testing integrating Sentry for error reporting, I noticed the error when forcing js to be enabled. (At some point we'll likely need js 🤷‍♂️)

[brookslybrand]

The issue seems to be that the server formats slightly differently than the client does. Specifically spaces are NARROW NO-BREAK SPACE instead of normal spaces

This is the best I could find on the issue: nodejs/node#45171. However, that concerns node 19 and says it's a bug 🤷‍♂️

Solutions I see:

• Handle the formatting in the server
• Use String.prototype.replaceAll() to replace the server-spaces with normal spaces
• Leverage a useEffect to replace the the string by calling the format on the client and holding the value in state
  • Personally, this is my least favorite option because a) it adds useState and useEffect we have to keep up with and b) on certain screen sizes this could have unintended consequences vs ensuring it is the same string for the server and the client

[grantnorwood]

NARROW NO-BREAK SPACE

Good catch! I was confused why the strings looked exactly alike, but figured the fix would be the same anyway.

And agreed, formatting on the server seems like the best route.

[colbywhite]

From what i can tell, there is a change with ICU. take the following code

const SPACE_CHAR_CODE = 32;
const NARROW_NO_BREAK_CHAR_CODE = 8239;

console.log("versions", { node: process.versions.node, icu: process.versions.icu });

const date = new Date("2023-03-01T12:00:00.000Z");
const formatter = new Intl.DateTimeFormat("en-US", {
  hour: "numeric",
  minute: "2-digit"
});
const formattedString = formatter.format(date);

function countCharCode(string, charCode) {
  let count = 0;
  for (let i = 0; i < string.length; i++) {
    if (string.charCodeAt(i) === charCode) {
      count++;
    }
  }
  return count;
}

const spaceCount = countCharCode(formattedString, SPACE_CHAR_CODE);
const narrowNoBreakCount = countCharCode(formattedString, NARROW_NO_BREAK_CHAR_CODE);

console.log(formattedString, { spaceCount, narrowNoBreakCount });

Running that with node 18 returns

versions { node: '18.14.2', icu: '72.1' }
6:00 AM { spaceCount: 0, narrowNoBreakCount: 1 }

Running that with node 16 returns

versions { node: '16.17.0', icu: '71.1' }
6:00 AM { spaceCount: 1, narrowNoBreakCount: 0 }

this project was moved to node v18 via #73 and thus inherently moved to ICU 72.

I think the relevant change in ICU 72 is

• In many formatting patterns, ASCII spaces are replaced with Unicode spaces (e.g., a "thin space").

it seems like this thin space is only used before the PM/AM. my other tweaks say that "real" spaces are used everywhere else i could see.

it's not clear to me how to figure out what icu most browsers are using, but i suspect the issue here is they're not using ICU 72.

options for moving forward and my two cents on each:

• don't use AM/PM

if you use a 24-hour clock and format it as Thu, Mar 2, 18:00 CST, this goes away. UX impact here tho, so i guess that's a question for whoever is product owner.

• go back to node 16/icu 71

this is a more direct solution. the problem is we opted in for thin spaces unknowingly. lets opt out. i'm not sure if there was a reason for node 18 (other than it's the latest and greatest) tho. was there a specific node 18 feature that was needed?

• stop using native Intl.DateFormat

Intl.DateFormat simply doesn't give you character-by-character control of formatting. most cases that fine, but maybe we're in an edge case where it's not and it's time to just use luxon

• remove date parsing/formatting from the client side so there's no hydration mismatch

this seems like what's being attempted on #84 but i would challenge that this kind of defeats the point of hydration. so it feels a bit odd to me, but not the end of the world. i would also push to move the convo to a broader framing: how do you handle dates when doing SSR/hydration? i'm not sure the best practice is "just don't hydrate dates at all". maybe it is. not sure. seems extreme to say that i can never do a new Date on the client side - even if i properly handle all the tz stuff - out of fear of a ICU mismatch that might put a near invisible to the user "thin" space in there. that triggers my spider sense as heavy handed, but maybe that is the right answer? 🤷

• do nothing

in theory browser's will move to ICU 72. and this all goes away. this do nothing approach should probably be coupled with some research on how browser's handle ICU to confirm this is a good strategy. (i don't know enough about how the browser is handling ICU versions)

another argument for do nothing is that this isn't a problem today. it's only a problem if you turn on js, which leads me to ...

• revisit What Are We Building? #1

you posit that this is a problem b/c "At some point we'll likely need js 🤷‍♂️". i'm not sure that's true. you discovered this b/c of some sentry-related work i gather? i've kind of had a question about whether or not sentry is needed for a project like this. it's not really a remix concept. skimming #1 again, the goals that stick out are SEO and facilitating things around the group that meetup.com doesn't do (content, sign up for talks). it's not clear to me what sentry adds to that. i wouldn't expect what amounts to a largely static blog to have something as sophisticated as sentry. and largely static blog likely doesn't need js.

i would vote to do nothing until it becomes a real problem and at that point see if the ICU thing has gone away by then. and if not finished by that time, either drop the am/pm or use luxon.

[colbywhite]

this shows firefox moving to 71, and i couldn't find a similar entry about it moving to 72.

[brookslybrand]

Super helpful writeup, thank you Colby! Appreciate all the suggestions and thoughts. Yeah, disallowing Intl.DateFormat on the client doesn't seem like a permanent solution. I mostly want to avoid shipping a date library to the client, which is why I wanted to use Intl.DateFormat. I'll keep thinking about this.

As for using node 18, I'd prefer for us to be using whatever node LTS is, which right now is 18. This doesn't seem like a big enough issue for us to drop node versions

[grantnorwood]

Thank you for the deep dive, @colbywhite! TIL a lot!

"At some point we'll likely need js 🤷‍♂️"

I just don't want to paint ourselves into a corner where we can't easily turn on js when we need it.

re: Sentry ...

Since it's a community project where others can deploy code to production, having some error monitoring in place will help us know if/when we break something, even if our site is super simple at the moment. Plus, I got us a free license as an OSS project, and it can also give us performance data and web vitals, which is a bonus 😎

[jaruesink]

for what it's worth, we use Luxon and like it a lot (and I'm pro javascript)

[brookslybrand]

Alright, finally sat down and read through this textbook @colbywhite produced (sincerest thanks, this was super helpful! 🙏)

I want to address each of the options:

don't use AM/PM

I'd rather not move to a 24 hour clock just to solve this issue. I think PM is still helpful (even though I assume most people wouldn't expect the even to be in the morning).

go back to node 16/icu 71

I'd prefer to stay on node 18, since it's LTS and it's better to keep up to date. Going backwards just for a single node/browser mismatch like this just seems like tackling the wrong problem

stop using native Intl.DateFormat
Definitely a valid solution. With Luxon though, shipping 21.6 kB of JS just for date formatting makes me pretty nervous. I prefer using a builtin like Int.DateFormat for the time being, until we're in a place where we have multiple date formatting needs that make it worth it to just ship a library. Even then, I'd love to see if we can find a smaller library, or at least validate if Luxon is tree-shakable.

Alternatively, we could use Luxon in the server only, but then we basically have to do what's mentioned in the next point, which Colby listed some good reasons against doing.

remove date parsing/formatting from the client side so there's no hydration mismatch

I totally agree with Colby's analysis. I think I was taking the wrong approach -- if we're shipping JS we should expect hydration to work. Having arbitrary tribal-knowledge rule like this is not ideal, it would be better if date formatting worked the same on the server as it does on the client

do nothing
While I think technically this is fine, I do worry about the warnings in the console for other devs. Warnings in the console a) can be distracting, b) will likely encourage others to try to solve this problem, and c) can encourage other warnings to be ignored, which we don't want.

revisit #1

Totally valid. I do think eventually we'll want JS, so I'm okay solving this for now. Mostly because I think I've come up with a solution I'm much happier with. Will post the PR in just a bit.

[brookslybrand]

See #87 for latest solution. Basically I think we should just replace the problem-character