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

Noscript tags no longer rendering components in 16.5.0 #15238

Open
zfletch opened this issue Mar 28, 2019 · 5 comments
Open

Noscript tags no longer rendering components in 16.5.0 #15238

zfletch opened this issue Mar 28, 2019 · 5 comments

Comments

@zfletch
Copy link

zfletch commented Mar 28, 2019

Do you want to request a feature or report a bug?

Bug

What is the current behavior?

Starting in version 16.4.3, the following code:

<noscript>
  <a href="/cat">Cat</a>
  <a href="/dog">Dog</a>
</noscript>

is being rendered in the browser as:

<noscript></noscript>

If the current behavior is a bug, please provide the steps to reproduce and if possible a minimal demo of the problem.

Prior to 16.4.3: https://codesandbox.io/embed/5mww4nzpwp

After 16.4.3: https://codesandbox.io/embed/6v8m4yo303

(The changes are not visible, but if you inspect element you can see that, in the first example, the links are being rendered, and in the second example they're not being rendered.)

What is the expected behavior?

It should render in the browser the same as in the code:

<noscript>
  <a href="/cat">Cat</a>
  <a href="/dog">Dog</a>
</noscript>

Which versions of React, and which browser / OS are affected by this issue? Did this work in previous versions of React?

See above, it was working in versions prior to 16.4.3. (I couldn't find a previous issue mentioning this bug. I think it could have been introduced in the fix for #11423)

Why is this a problem?

I use a snapshot tool with React to generate a set of static pages from a React project. These pages have less functionality than the full application, but they allow webcrawlers and users who have disabled JavaScript to use the site at a basic level. For example, the code might look like this:

<FancyInteractiveButton linksTo="page">link</FancyInteractiveButton>
<noscript>
  <a href="page">link</a>
</noscript>

Preventing components in <noscript> tags from rendering breaks this functionality for users with JavaScript disabled. The generated snapshots no longer contain the links. It also makes the site harder to navigate by webcrawlers, even if they have JavaScript enabled, because they have to be smart enough to use the fancy button instead of following the link.

@gaearon
Copy link
Collaborator

gaearon commented Mar 29, 2019

This change was intentional I think: #11423.

As a workaround, maybe this works for you?

        <noscript
          dangerouslySetInnerHTML={{
            __html: '<a href="page">link</a>'
          }}
        />

?

@zfletch
Copy link
Author

zfletch commented Mar 29, 2019

@gaearon thanks for the response!

Unfortunately in my use case dangerouslySetInnerHTML doesn't work (without modifying the code) since it expects a string and not a React component.

Consider the code:

const Links = () => (
  <div>
    <a href="/cat">Cat</a>
    <a href="/dog">Dog</a>
  </div>
);

const Component = () => (
  <Fragment>
    <FancyToggleOnClick element={Links}>
      Toggle the links
    </FancyToggleOnClick>

    <noscript>
      <Links />
    </noscript>
  </Fragment>
);

Prior to 16.4.3, this worked fine: the HTML generated by React included the links in the <noscript> tag, a snapshot of the page was taken and served as plain HTML which was hydrated client-side, and users with and without JavaScript could use the application.


I read #11423 in more detail and it does seem like a pretty complicated issue. I can see why that solution was preferred. I do have two points to add:

  1. the change may have fixed some issues for people using a Node server, but it simultaneously broke <noscript> for anyone using a prerendering tool like react-snap, Prerender SPA Plugin, or any of the alternatives.

  2. the change was made in a patch version (16.4.2 -> 16.4.3) even though it changed the behavior of <noscript> pretty significantly. I think in the future large changes to <noscript> should go in at least a minor release. The change was included in 16.5.0, there is no 16.4.3

@zfletch zfletch changed the title Noscript tags no longer rendering components in 16.4.3 Noscript tags no longer rendering components in 16.5.0 Mar 29, 2019
@Ephem
Copy link
Contributor

Ephem commented Apr 18, 2019

I just noticed this issue and as the one who caused it I would like to chip in, first of all, sorry for the trouble. 😄

I don't think anyone (at least not me), were considering prerenderers in #11423 which is unfortunate, breaking this use case should of course have been an intentional choice rather than an unintentional side effect.

That said, I do still think the fix is a correct one. Using JS to create markup that will never be used in the current environment seems wasteful to the vast majority of targets that run this code. The dangerouslySetInnerHTML-workaround seems like a valid escape hatch when it is indeed needed?

If you need to put the result of a rendered component into a <noscript> in the browser, it might be possible to borrow a trick that was sometimes used on the serverside before #11423, using ReactDOMServer.renderToString to generate the markup. This isn't a great solution obviously.

All this said, if this use case is strong enough it should be possible to introduce a special prop on <noscript>-components that control this behaviour. Something like <noscript alwaysRenderNoScriptContents>...</noscript>. Not sure if this is desirable or not in the big picture. 🤷‍♂️ If it is, I could take a stab at implementing it.

@designreact
Copy link

designreact commented Aug 18, 2020

I've been struggling with this aswell, but we have a slightly different usecase.

Our project leverages noscript for server markup intended for SEO crawlers. Then after the app is running clientside we use react-intersection-observer to infill the DOM. This preserves crawlability for some less advanced search engines and markedly improves Lighthouse's excessive DOM depth scores.

I'm not an SEO expert by any stretch so the above might be overkill but I do think this is fairly standard practice?

@Ephem
Copy link
Contributor

Ephem commented Aug 18, 2020

@designreact Without knowing your exact usecase that sounds.. Complex. I'm not sure exactly what you are doing, but it sounds like the noscript-part of that should work since it's generating the content serverside with renderToString?

So just to clear up any confusion, I'll try to summarize how this should work today (though I haven't tested in a while):

function NoscriptFallback() {
  return 'I will be shown only when JS is disabled';
}

function Component() {
  return (
    <noscript>
      <NoscriptFallback />
    </noscript>
  );
}

Server

The above code should work as intended when running on the server with ReactDOMServer.renderToString(), that is, if you turn off JS in the browser, you should see that message. Even if you don't disable JS, if you inspect the markup, you should see that message inside of a <noscript>-tag. (Note that I'm not sure if it shows up in the devtools Elements-pane, you might have to view source)

Hydration

If you do ReactDOM.hydrate() on the above code, the <NoscriptFallback />-component should never run, but React leaves the existing content inside the noscript (delivered by the server-render) untouched.

Client only

If you do ReactDOM.render() on the above code without having server-rendered anything, the <NoscriptFallback /> will never run and no content will be generated. The reasoning here is that JS is already running, so that content will never be shown anyway. (Turns out this is not true for pre-renderers running in the browser as the original issue pointed out)

TL;DR: Content gets generated inside of <noscript> on the server, but not on the client, though it does get preserved when hydrating server markup.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants