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.

Sign up for GitHub

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

Jump to bottom

TypeError: Cannot read property 'join' of undefined #910

Closed
ooade opened this issue Jan 28, 2017 · 19 comments
Closed

TypeError: Cannot read property 'join' of undefined #910

ooade opened this issue Jan 28, 2017 · 19 comments
Assignees
@timneutkens
Labels
Upstream Related to using Next.js with a third-party dependency. (e.g., React, UI/icon libraries, etc.).

Comments

@ooade
Copy link
Contributor

ooade commented Jan 28, 2017 •
edited
Loading

This occurs only when including preact.

Steps to reproduce:

  • Run the preact example app
  • Add nextjs's Head helper element to a page
  • Give the page a title; then that happens.

The problem arose from the header-manager's method(updateTitle). Preact's VNode exposes just attributes to props with no children, the VNode itself exposes children.
@ooade ooade mentioned this issue Jan 28, 2017
Closed
@timneutkens timneutkens self-assigned this Jan 28, 2017
@timneutkens
Copy link
Member

I'll have a look 👍

@timneutkens
Copy link
Member

Ok I get what you mean. We read component.props.children which is set in React, but not in preact. in preact it's component.children Looks like something that would have to be added to preact-compat. I'll check that now 👍

@timneutkens
Copy link
Member

Looks like it's handled here already 🤔 https://github.com/developit/preact-compat/blob/master/src/index.js#L109

@developit could you have a look? ❤️

@timneutkens timneutkens added Upstream Related to using Next.js with a third-party dependency. (e.g., React, UI/icon libraries, etc.). Status: In Progress labels Jan 28, 2017
@developit
Copy link
Contributor

developit commented Jan 28, 2017 •
edited
Loading

@timneutkens I just checked and I'm not sure I can reproduce the above issue - here's the JSFiddle I set up: https://jsfiddle.net/developit/n144pt7m/

I might be going about the repro wrong though!

FWIW I'm not sure L45 of <HeadManager> accounts for the case where a single element is passed. In React (and thus in preact-compat), that will result in props.children being a reference to the Virtual DOM Element, rather than an Array.

That is to say, this line:

typeof children==='string' ? children : children.join('')

...if rendered like this:

<HeadManager><AnyThing /><HeadManager>

... will throw an exception, because children is an Object (VNode), not a String or Array.

It seems like this would be an issue when running under React as well, since preact-compat's behavior here is just mimicking React's. Adding an Array check before the join would at least circumvent any exception.

@timneutkens
Copy link
Member

timneutkens commented Jan 28, 2017 •
edited
Loading

That is a fair point, we will have to update that. The most minimal example where I can reproduce this is this: https://github.com/timneutkens/next-issue-910. In this case, props.children is undefined. That's why it logs an error.

@developit
Copy link
Contributor

Ah perfect @timneutkens that gives me a great repro. It looks like the bit that adds props.children is only being applied to Component vnodes, not element VNodes. We can remedy that quickly :)

@timneutkens
Copy link
Member

Much appreciated ❤️

developit added a commit to preactjs/preact-compat that referenced this issue Jan 28, 2017
@developit
Fix children normalization for element VNodes (vercel/next.js#910)
1fdf4fa
@developit
Copy link
Contributor

Should be fixed in [email protected]

@timneutkens
Copy link
Member

Thank you very much @developit 👍 💯

@ooade
Copy link
Contributor Author

ooade commented Jan 28, 2017

Thanks @developit

@developit
Copy link
Contributor

Just confirmed using your repro (thanks again btw!) that it is in fact working now:
screen shot 2017-01-28 at 6 12 20 pm

@timneutkens
Copy link
Member

@developit confirmed it too ❤️

@developit
Copy link
Contributor

Great, thanks a million for surfacing this Tim!

@tgoldenberg tgoldenberg mentioned this issue Jun 6, 2017
Closed
@atuttle
Copy link

atuttle commented Sep 26, 2017 •
edited
Loading

I am seeing this with:

  • React 15.6.1
  • React DOM 15.6.1
  • Next.js 3.2.2

I want to have a page title containing a bullet character (&bull;). If I do:

<Head><title>foo &bull; bar</title></Head>

... then the title appears as "foo &bull; bar" because the &bull; gets escaped to &amp;bull;.

When I switch to:

<title dangerouslySetInnerHTML={{ __html: "foo &bull; bar" }} />

... the title is set correctly, but this error starts appearing in the browser console, ostensibly because <title></title> has no children at all?

@atuttle
Copy link

atuttle commented Nov 9, 2017

ping @timneutkens @rauchg

@donaminos
Copy link

@atuttle I am running on the same issue with updating title tag. Did you find a fix or workaround please ? tanks in adavnce

@Irrelon
Copy link

Irrelon commented May 2, 2018

I'm seeing this issue whenever I navigate on the client-side. If I do a refresh it doesn't occur and the title of the page is correct.

@atuttle
Copy link

atuttle commented Jun 25, 2018

@donaminos no, no solution or workaround has been provided yet. It's possible that this is not a problem in a more recent version of Next, but I haven't had the chance to upgrade yet. I am just not using any HTML entities in my title, as annoying as that is.

@danjebs
Copy link

danjebs commented Oct 11, 2018

Does this have any ramifications, or can it be treated as a warning for now? I am using dangerouslySetInnerHTML to unescape special characters in the page's server render, which I suppose isn't a huge deal if I were to stop using it. But I'd prefer to have nice clean titles for the Google.

@lock lock bot locked as resolved and limited conversation to collaborators Oct 11, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@timneutkens timneutkens

Labels
Upstream Related to using Next.js with a third-party dependency. (e.g., React, UI/icon libraries, etc.).
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
7 participants
@atuttle @developit @Irrelon @donaminos @timneutkens @ooade @danjebs