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

Fragments not supported in <Head /> #3399

Closed
1 task done
tusbar opened this issue Dec 5, 2017 · 4 comments
Closed
1 task done

Fragments not supported in <Head /> #3399

tusbar opened this issue Dec 5, 2017 · 4 comments

Comments

@tusbar
Copy link
Contributor

tusbar commented Dec 5, 2017 •
edited
Loading

  • I have searched the issues of this repository and believe that this is not a duplicate.

Expected Behavior

Considering the following snippet:

<Head>
  {title ? (
    <React.Fragment>
      <title>{title} | Cool site</title>
      <meta property='og:title' content={title} />
    </React.Fragment>
  ) : (
    <title>Cool site</title>
  )}

  {/* … */}
</Head>

When title is defined, I would expect <title>foo | Cool site</tite> to be rendered.

Current Behavior

An empty <title /> is rendered.

Your Environment

Tech Version
next 4.1.4
node 8.9.1
@loris
Copy link

loris commented Dec 14, 2017

Confirm same behavior, it seems to be only emptying the title tag. Other type of head tags (meta, link, etc.) are correctly rendered.

@arunoda
Copy link
Contributor

arunoda commented Dec 14, 2017

Actually what's happening inside <Head/> is quite different than other elements. We read elements(direct children) inside in the Head add it to the actual head.

In this case, we only see the React.Fragment but not the content inside it.

So, with conditional logic, you can as have as many as <Head> tags rather implementing logic inside it.

But if someone willing to add React.Fragment support, we are super glad to take it.

@Sekhmet
Copy link
Contributor

Sekhmet commented Dec 25, 2017 •
edited
Loading

Hey,
I was super excited to start contributing and decided to try to solve this issue. I have come up with following change:

diff --git a/lib/head.js b/lib/head.js
index 3473332..e9842f8 100644
--- a/lib/head.js
+++ b/lib/head.js
@@ -21,6 +21,12 @@ function reduceComponents (components) {
   .map((c) => c.props.children)
   .map((children) => React.Children.toArray(children))
   .reduce((a, b) => a.concat(b), [])
+  .reduce((a, b) => {
+    if (React.Fragment && b.type === React.Fragment) {
+      return a.concat(React.Children.toArray(b.props.children))
+    }
+    return a.concat(b)
+  }, [])
   .reverse()
   .concat(...defaultHead())
   .filter((c) => !!c)

This makes code posted by tusbar work without problems. This doesn't change current logic much - it's still reading direct children (so no nested Fragments).

Is it kind of solution you are looking for? If it is I'm happy to take this issue and create PR.
cc @arunoda

@tusbar
Copy link
Contributor Author

tusbar commented Dec 26, 2017

@Sekhmet that looks good. :) Open that PR, it’ll get more visibility.

@tusbar tusbar closed this as completed Dec 26, 2017
@tusbar tusbar reopened this Dec 26, 2017
@Sekhmet Sekhmet mentioned this issue Dec 26, 2017
Merged
@lock lock bot locked as resolved and limited conversation to collaborators Dec 28, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@loris @arunoda @tusbar @Sekhmet