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

<Head> do not work as expected if not at the document/page level #5038

Closed
SBoudrias opened this issue Aug 27, 2018 · 8 comments · Fixed by #5068
Closed

<Head> do not work as expected if not at the document/page level #5038

SBoudrias opened this issue Aug 27, 2018 · 8 comments · Fixed by #5068

Comments

@SBoudrias
Copy link
Contributor

Bug report

Describe the bug

I think this is a regression caused by the refactor inside #5020 (v7 canary-3). We tested going back to canary-1 and the bug wasn't there.

Basically, the issue is that style elements (with key) inside a <Head> that is not injected in the _document.js file or the pages/* level are not working.

The issue seems to be quite consistent per pages, but not consistent per types of components. Sometime it work and sometime it doesn't. The only place it seemed to consistently failed was inside dynamic() components.

Expected behavior

next/head should always inject the content in the head of the page no matter where it's been called (dynamic imports, portals, component level, etc)

System information

  • Version of Next.js: v7 canary-3
@SBoudrias
Copy link
Contributor Author

I wonder if this bug could be related to some older issues reported:

@timneutkens
Copy link
Member

Could you create a minimal reproduction in a GitHub repository? Thanks 🙏

@timneutkens
Copy link
Member

Hey Simon,
It seems like the link is missing

@SBoudrias
Copy link
Contributor Author

@timneutkens oops, posted that by mistake. I'll try to provide you with an minimal reproduction. Can't use npm right now because of an SSL certificate issue, I'll try to get something working as soon as possible.

@timneutkens
Copy link
Member

Great! Thanks 🙏

@SBoudrias
Copy link
Contributor Author

@timneutkens okay, got a repro pushed here: https://github.com/SBoudrias/bug-next-head

In this minimal reproduction, a green box will appear properly with Next 6, but it doesn't show with Next 7.

@timneutkens
Copy link
Member

@SBoudrias amazing reproduction, that you very much, this allowed me to track it down way faster. I've also used the reproduction as the test to see if it works correctly: https://github.com/zeit/next.js/pull/5068/files#diff-5fa00d00bca1d3713b89864f75f55750

This PR should fix exactly what you were experiencing.

@SBoudrias
Copy link
Contributor Author

Awesome! Thanks a lot for looking into this :)

timneutkens added a commit that referenced this issue Sep 2, 2018
Fixes #5038

The problem with `constructor` is that it doesn't have `context` yet when being called. It's also considered unsafe to add a side-effect on constructor except when server-rendering
@lock lock bot locked as resolved and limited conversation to collaborators Sep 2, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants