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

<Head /> doesn't wait for assets to finish loading #1011

Closed
matthewmueller opened this issue Feb 6, 2017 · 8 comments
Closed

<Head /> doesn't wait for assets to finish loading #1011

matthewmueller opened this issue Feb 6, 2017 · 8 comments

Comments

@matthewmueller
Copy link
Contributor

If you have a <link> tag in your head and you do a Router.push(...), I've noticed an occasional flash of unstyled content.
@nkzawa
Copy link
Contributor

nkzawa commented Feb 6, 2017

I think this occurs only on Safari ? Please use <style jsx global /> instead of <Head><style/></Head> in that case.

Related: vercel/styled-jsx#102

@matthewmueller
Copy link
Contributor Author

matthewmueller commented Feb 6, 2017 •
edited
Loading

Happening on latest chrome, it's actually a <link /> tag. Here's an example:

<Head>
    <title>{me.firstName} {me.lastName} Website</title>
    <link rel='shortcut icon' type='image/x-icon' href='/static/assets/favicon.ico' />
    <link href='https://fonts.googleapis.com/css?family=Assistant:200,300,400,600,700,800|Lora:400,400i,700,700i' rel='stylesheet' />
    <link href='/static/styles/normalize.css' rel='stylesheet' />
    <link href='/static/styles/spacing.css' rel='stylesheet' />
    <link href='/static/styles/layout.css' rel='stylesheet' />
    <link href='/static/styles/theme.css' rel='stylesheet' />
    <link href='/static/styles/global.css' rel='stylesheet' />
    <link href='/static/styles/footer.css' rel='stylesheet' />
    <meta name='viewport' content='width=device-width, initial-scale=1' />
</Head>

Haven't tried adding keys or anything, but looking at the source, it doesn't look like it waits for any assets to load before transitioning.

@nkzawa
Copy link
Contributor

nkzawa commented Feb 7, 2017 •
edited
Loading

This is a bug. I think we should render particular tags like link with no delay, and others like title have to be updated in batch.

But basically, I think link in Head is not recommended since these files would be downloaded again and again when you navigate to pages multiple times.

@matthewmueller
Copy link
Contributor Author

matthewmueller commented Feb 7, 2017 •
edited
Loading

This is a bug. I think we should render particular tags like link with no delay, and others like title has to be updated in batch.

Awesome, I'm happy to hear that since linking stylesheets is the simplest and often the fastest way to put together a website.

But basically, I think link in Head is not recommended since these files would be downloaded again and again when you navigate to pages multiple times.

Yah, I can't say I've tested this out, but I wonder if it could either check if that css file already exists (in the side-effect) or perhaps use key={...} to ensure that it doesn't get re-rerendered (and probably re-fetched).

@nkzawa
Copy link
Contributor

nkzawa commented Feb 7, 2017 •
edited
Loading

Tags don't get re-rendered if already exist. The problem is they are removed if same tags are not included in other pages. So I rather recommend to have link on Custom Document. (or make sure to include same <Head> among all pages)

@matthewmueller
Copy link
Contributor Author

matthewmueller commented Feb 7, 2017 •
edited
Loading

Ahh got it, that makes sense that the <Head/> would clean up after itself. Hm, yah maybe a custom document is the way to go. I'm also wondering if this could be solved with a <Link /> component, I guess they would somehow need to tie into the router though to wait for the asset to finish before the transition.

@matthewmueller
Copy link
Contributor Author

matthewmueller commented Feb 8, 2017 •
edited
Loading

Just tried out the custom <Document />. Works seamlessly! I think this is a valid/easy solution to external stylesheets! May just want to make a note in the docs about it. I'm going to close, but feel free to reopen if there's a bug you'd like to address.

sedubois added a commit to sedubois/relate that referenced this issue Feb 13, 2017
@sedubois
Move Head to custom document to ensure is the same across all pages (v…
db8ce49 
…ercel/next.js#1011)
@Kikobeats
Copy link
Member

@matthewmueller hey, can you paste your final solution? 😄

@lock lock bot locked as resolved and limited conversation to collaborators May 11, 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
3 participants
@matthewmueller @nkzawa @Kikobeats