-
Notifications
You must be signed in to change notification settings - Fork 30.5k
Description
Bug report
next/head provides much of the same functionality contained in react-helmet. Helmet is unsafe to use with SSR because of its reliance on react-side-effect, which holds state in what is essentially a global variable. next/side-effect appears to be react-side-effect with only slight modifications.
Any SSR pipeline that includes asynchronous elements is susceptible to data corruption when a Node server, like Express, is handling multiple requests asynchronously. I wrote about this here: https://open.nytimes.com/the-future-of-meta-tag-management-for-modern-react-development-ec26a7dc9183
The TL;DR is that <Helmet> (and probably <Head>) artifacts need to be scoped to each request, not tied to a global variable exposed by withSideEffect(). react-helmet-async accomplishes this via a <HelmetProvider>.
Without scoping the artifacts, pages can end up with meta tags for a different request. At The New York Times, we discovered this when reporters posted to Twitter, but the articles linked to a different story (because the meta tags were from a different request)! This is occurs when Helmet.renderStatic() or Helmet.rewind() are called while 2 requests are in flight at the same time.
To reproduce
Take an arbitrary list of URLs and save them in urls.csv. Run the following bash script in 2 separate terminal windows, which should cause requests to queue up.
#!/bin/sh
INPUT=urls.csv
while read httpRequest_requestUrl
do
random_url=”http://localhost:3000${httpRequest_requestUrl}"
echo “\nRequesting ${random_url}”
curl -s ${random_url} | grep -ioE “<title[^>]+>[^<]*<\/title>”
done < $INPUT
This will mainly occur on apps that have an asynchronous data hydration step, like GraphQL-based apps that use Apollo.
I do not have a patch ready for this, but I am interested to hear thoughts around this.