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

_document -> makeStylesheetInert -> "Cannot read property 'type' of null" if Child is null #24783

Closed
pecoram opened this issue May 4, 2021 · 15 comments
Assignees
Labels
Font (next/font) Related to Next.js Font Optimization. please add a complete reproduction The issue lacks information for further investigation

Comments

@pecoram
Copy link

pecoram commented May 4, 2021

What version of Next.js are you using?

10.2.0

What version of Node.js are you using?

14.16.0

What browser are you using?

All

What operating system are you using?

Windows

How are you deploying your application?

Other platform

Describe the Bug

When a Child of the Head is null an exception is thrown: TypeError: Cannot read property 'type' of null.

In the condition a nullsafe is missing

c.type === 'link' &&

Expected Behavior

if (c && c.type === 'link' &&
        c.props['href'] &&
        OPTIMIZED_FONT_PROVIDERS.some((url) => c.props['href'].startsWith(url))
      ) 

To Reproduce

In the _document put a null component inside the Head.

import Document, { Html, Head, Main, NextScript } from 'next/document'

const NullComponent = () => null;

class MyDocument extends Document {
  static async getInitialProps(ctx) {
    const initialProps = await Document.getInitialProps(ctx)
    return { ...initialProps }
  }

  render() {
    return (
      <Html>
        <Head>
            <NullComponent />
        </Head>
        <body>
          <Main />
          <NextScript />
        </body>
      </Html>
    )
  }
}

export default MyDocument
@pecoram pecoram added the bug Issue was opened via the bug report template. label May 4, 2021
@ljosberinn
Copy link
Contributor

Can you elaborate why you're rendering a NullComponent in Head? Head should contain regular head-elements such as link, script, meta, not nothing.

@pecoram
Copy link
Author

pecoram commented May 5, 2021

for example when you write something like that:

{ condition && <link rel="preload" as="image" href="" /> }
or

const Component = () => <link rel="preload" as="image" href="" />;
{ condition && <Component /> }

if condition is false you get the error!

@pecoram
Copy link
Author

pecoram commented May 5, 2021

now I solved it this way:

{condition ? <Component /> : ('')}

but a nullsafe in that point is a better and safer solution!

@ljosberinn
Copy link
Contributor

agreed, I'll try to PR this later today

@synoptase
Copy link

synoptase commented May 5, 2021

EDIT:
My code was wrapping a <><link /></> with a fragment, therefore being read only. Fixed my issue.

Still a bit related to the original issue. accessing props maybe needs more sanity checks? (nullsafe, readonly...)

EDIT#2:
Seems env related, not a bug for me. (#24783 (comment))


I'm having a somewhat similar issue: TypeError: Cannot assign to read only property 'children' of object '#<Object>'
Traced back to:

c.props['children'] = this.makeStylesheetInert(c.props['children'])

nextjs 10.2.0
node 12.22.1

Still trying to figure out the root cause though.

@ljosberinn
Copy link
Contributor

ljosberinn commented May 5, 2021

@pecoram could you try to create a reproducible repo?

Both

import Head from "next/head";

export default function Test() {
  return (
    <>
      <Head>{false && <link rel="preload" as="image" href="" />}</Head>
      <h1>hi</h1>
    </>
  );
}

and

import Head from "next/head";

export default function Test() {
  return (
    <>
      <Head>{null}</Head>
      <h1>hi</h1>
    </>
  );
}

work flawlessly for me

@synoptase same for you please,

import Head from "next/head";

export default function Test() {
  return (
    <>
      <Head>
        {/* // eslint-disable-next-line react/jsx-no-useless-fragment */}
        <>{null}</>
      </Head>
      <h1>hi</h1>
    </>
  );
}

works fine too

@synoptase
Copy link

synoptase commented May 6, 2021

@ljosberinn it worked indeed in my dev env. But once build and deployed, error kept appearing until i removed Fragment. It definitely seems related to the build env, but i just tried reproducing the issue by building and running my docker image locally, without any luck.

Not a bug as initially reported by my first comment then.

@timneutkens timneutkens added kind: bug and removed bug Issue was opened via the bug report template. labels May 6, 2021
@timneutkens timneutkens added this to the Iteration 20 milestone May 6, 2021
@timneutkens
Copy link
Member

This is a bug, it's related to the new font optimization (which only runs during build).

@ljosberinn
Copy link
Contributor

I believe changing the implementation of _document.makeStylesheetInert to this should fix it but haven't found a way to test it integratively:

  makeStylesheetInert(node: ReactNode): ReactNode[] {
    return React.Children.map(node, (child) => {
      if (!React.isValidElement(child)) {
        return null
      }

      if (
        child.type === 'link' &&
        child.props.href &&
        OPTIMIZED_FONT_PROVIDERS.some((url) => child.props.href.startsWith(url))
      ) {
        return React.cloneElement(child, {
          ...child.props,
          'data-href': child.props.href,
          href: undefined,
        })
      }

      if (child.props?.children) {
        return React.cloneElement(child, {
          ...child.props,
          children: this.makeStylesheetInert(child.props.children),
        })
      }

      return child
    })
  }

@fbudassi
Copy link

fbudassi commented May 6, 2021

I'm also experiencing a similar problem:
Node.js: 14.16.0
Next.js: 10.2.0

Error message:
TypeError: Cannot assign to read only property 'children' of object '#<Object>'

But in my case, this is the line inside the _document.tsx's <Head> that triggers the problem:

<style>{`#__next { height: 100%; }`}</style>

It wasn't failing in Next.js 10.0.6.

@janicklas-ralph
Copy link
Contributor

I'm having a hard time reproducing this bug. We also already check for this edge case

Could you make a small example where the error is reproducible?

@timneutkens timneutkens added the please add a complete reproduction The issue lacks information for further investigation label May 23, 2021
@flybayer
Copy link
Contributor

flybayer commented Jun 3, 2021

Error message:
TypeError: Cannot assign to read only property 'children' of object '#<Object>'

Maybe there's two bugs here, but I get this error message with NODE_ENV=test next build which is this bug: #25491

@atdrago
Copy link

atdrago commented Jun 24, 2021

Our project suddenly started returning 500s in production because of TypeError: Cannot assign to read only property 'children' of object '#<Object>'.

I've created a project where I've reproduced the issue (https://github.com/atdrago/repro-24783). Here's the commit where the issue is introduced: atdrago/repro-24783@1ea71f6

Steps to reproduce:

  1. npm i && npm run build && npm start
  2. Navigate to http://localhost:3000/foo/bar (/foo/bar isn't special, it can be /foo/anything-here)
  3. Observe the 500 error

Some things I noticed:

Node.js: 14.15.0
Next.js: 11.0.1

Please let me know if you have any questions, and thank you for your time and effort looking into this issue!

I've created a related issue on next-boost here: next-boost/next-boost#42

@timneutkens timneutkens added the Font (next/font) Related to Next.js Font Optimization. label Nov 16, 2021
@timneutkens timneutkens removed this from the 12.0.5 milestone Nov 17, 2021
@huozhi
Copy link
Member

huozhi commented Nov 29, 2021

It's fixed in next-boost itself in v0.10.2.

@huozhi huozhi closed this as completed Nov 29, 2021
kodiakhq bot pushed a commit that referenced this issue Nov 30, 2021
x-ref: #28498, #31784

I can repro the issue #24783 with `next-boost` 0.10.1 and it was fixed in 0.10.2 (ref: next-boost/next-boost@eba6d10). The actual case is missing setting node env to `"production"`.

React freeze element props and element itself in dev mode (ref: https://github.com/facebook/react/blob/a724a3b578dce77d427bef313102a4d0e978d9b4/packages/react/src/ReactElement.js#L194-L196)

Then next.js will reassign props with react development bundle while next-boost is enabled. Those operations are only happened in non-dev mode so it's good to remove now.

This PR + #31898 == revert #31784

cc @styfle @awareness481
@balazsorban44
Copy link
Member

This issue has been automatically locked due to no recent activity. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you.

@vercel vercel locked as resolved and limited conversation to collaborators Jan 27, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Font (next/font) Related to Next.js Font Optimization. please add a complete reproduction The issue lacks information for further investigation
Projects
None yet
Development

No branches or pull requests