-
Notifications
You must be signed in to change notification settings - Fork 27.1k
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
Documentation about Head.rewind #9326
Comments
We can get rid of |
@timneutkens Could you explain what implications this has from a refactoring perspective? |
@timneutkens In addition, could you clarify what you mean by legacy context? Is there any other aspects of the code that we need to refactor with this change? I also see that |
Very new to Next. My guess based on this thread, the name of the method, and examples I've seen: It's probably got to do with how Head is populated as a side-effect of rendering. In the examples with Head.rewind I've seen, the method is called after rendering that is meant to compute properties for rendering. I'm guessing this undoes changes to Head caused during such renders so it can be populated later. |
@iFallUpHill I have done some digging, and it seems that |
This has apparently been unnecessary since [email protected]: vercel/next.js#9326 (comment)
@jaydenseric Head.rewind is currently still needed if you do tree rendering, eg getDataFromTree in Apollo Would highly recommend not doing tree rendering as it has massive performance implications. |
This is surprising to hear, considering it was deliberately removed from the Next.js Apollo example: 1a50d99#diff-c7ee3d6e815d630e8d7be23ea73b6436L107 Are you sure it is necessary; is your earlier comment and the Apollo example now incorrect?
I know all about tree rendering, I've worked with the Apollo one and published before using several different techniques:
Are you able to offer an alternative for server side rendering nested query components? |
Yes I'm pretty sure it's necessary, so yeah the example should be updated. My comment is still correct, I said "We can get rid of it" haven't had the time to do so because of other priorities.
Extracting the data requirements out of the React tree like Relay does and then executing it in getStaticProps/getServerProps (new data fetching methods) |
This reverts commit 142f1a2. Apparently Head.rewind() is necessary after all: vercel/next.js#9326 (comment)
Update: There aren't any examples using |
It's not being removed btw, #14091 is backwards compatible |
And now it is 2021, and I found one in the legacy code, while updating and refactoring. Just removing it from apollo.tsx typescript complains that rewind does not exist on Head, which has type |
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. |
Feature request
Is your feature request related to a problem? Please describe.
I see
Head.rewind()
being used in several examples (e.g. the apollo example) but I don't really understand what it does and can't find documentation about it.Describe the solution you'd like
If possible, I would like some documentation about it. What happens if I omit it? Could it be possible to have a concrete example?
The text was updated successfully, but these errors were encountered: