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

JS modules are loaded twice #11107

Closed
Reggino opened this issue Mar 16, 2020 · 24 comments
Closed

JS modules are loaded twice #11107

Reggino opened this issue Mar 16, 2020 · 24 comments

Comments

@Reggino
Copy link
Contributor

Reggino commented Mar 16, 2020

Bug report

Describe the bug

JS modules seem to be loaded twice

To Reproduce

Steps to reproduce the behavior:

  1. Go to 'https://www.nextjs.org' (or your own built site)
  2. Click on 'developer console' in e.g. Chrome
  3. See Network tab
  4. Do a hard refresh (E.g. CTRL + SHIFT + R)
  5. Sort resources by name
  6. Look at loaded JS modules. They appear twice in the resource list, but are not retrieved from cache

Expected behavior

All required resources to be loaded just once.

Screenshots

image

System information

  • OS: Ubuntu Linux
  • Browser Chrome
  • Version of Next.js: 9.1.1 - 9.3.0

Additional context

When disabling granular loading in next.js config, the issue seems resolved.

    experimental: {
      granularChunks: false,
      css: false
    }
@Timer
Copy link
Member

Timer commented Mar 16, 2020

Can you turn off your experimental usage of the modern behavior and see if this persists?

@Reggino
Copy link
Contributor Author

Reggino commented Mar 16, 2020

Can you turn off your experimental usage of the modern behavior and see if this persists?

The issue seems resolved using the configuration:

    experimental: {
      modern: false
    }

@Reggino Reggino changed the title Resources loaded twice due to granular chunks behaviour JS modules are loaded twice Mar 16, 2020
@Reggino
Copy link
Contributor Author

Reggino commented Mar 16, 2020

(I've updated the title, since my assumption of the root cause may be incorrect ;-) )

@cansin
Copy link

cansin commented May 9, 2020

I am having the exact same problem and do not have my experimental config turned on. According to the sourcemap, there are 2 places loading the JS files:

// at page-loader.js
.
.
.
function appendLink(href, rel, as) {
    return new Promise((res,rej,link)=>{
        link = document.createElement('link');
        link.crossOrigin = process.crossOrigin;
        link.href = href;
        link.rel = rel;
        if (as)
            link.as = as;
        link.onload = res;
        link.onerror = rej;
        document.head.appendChild(link); // <--- Here
    }
    );
}
.
.
.

and at document root (i.e. the HTML served) the actual <link rel="preload" ... /> tag. I am not sure when this started happening though. Ideas? I can try to create a small reprod repo if you want.

Screenshot 2020-05-09 22 54 00

@cansin
Copy link

cansin commented May 9, 2020

I am not sure if this is related but the reported First Load JS sizes are also roughly twice the size of First Load JS shared by all (which looks suspicious/weird):

Page                                                           Size     First Load JS
┌ λ /                                                          1.73 kB         227 kB
├   /_app                                                      5.92 kB         105 kB
├ ○ /404                                                       769 B           210 kB
├ λ /forgot-password                                           1.32 kB         227 kB
├ λ /login                                                     1.33 kB         227 kB
├ ○ /offline                                                   1 kB            211 kB
├ λ /reset-password                                            2.37 kB         228 kB
├ λ /signup                                                    1.31 kB         227 kB
└ λ /todos                                                     3.11 kB         228 kB
+ First Load JS shared by all                                  105 kB
  ├ static/pages/_app.js                                       5.92 kB
  ├ chunks/3704887d5176932cc40c267af232f229c1b773b4.e8a25d.js  41.7 kB
  ├ chunks/commons.f7f234.js                                   10.5 kB
  ├ chunks/framework.126679.js                                 40.4 kB
  ├ runtime/main.89a34b.js                                     6.02 kB
  └ runtime/webpack.c21266.js                                  794 B

@cansin
Copy link

cansin commented May 9, 2020

And here is my next.config.js in case it helps:

const dotenv = require("dotenv");
const withSourceMaps = require("@zeit/next-source-maps");
const withSitemap = require("next-with-sitemap");

dotenv.config();

module.exports = withSourceMaps()(
  withSitemap({
    compress: true,
    env: {
      GQL_URL: process.env.GQL_URL,
      GQL_AUTHORIZATION: Buffer.from(
        `${process.env.GQL_CLIENT_ID}:${process.env.GQL_CLIENT_SECRET}`
      ).toString("base64"),
      GTM_ID: process.env.GTM_ID,
      GTM_AUTH: process.env.GTM_AUTH,
      GTM_PREVIEW: process.env.GTM_PREVIEW,
    },
    pageExtensions: ["page.js"],
    sitemap: {
      baseUrl: `${
        process.env.SECURE_SSL_REDIRECT !== "false" ? "https" : "http"
      }://${process.env.WEB_DOMAIN}`,
    },
  })
);

and some of my library versions:

    "@apollo/react-hooks": "^3.1.5",
    "@zeit/next-source-maps": "^0.0.4-canary.1",
    "next": "^9.3.6",
    "next-with-apollo": "^5.0.1",
    "react": "^16.13.1",
    "react-dom": "^16.13.1",

@cansin
Copy link

cansin commented May 9, 2020

I want to speculate this issue might be related with #9070

@cansin
Copy link

cansin commented May 9, 2020

I have also realized the issue does not happen for (Static) pages:

Screenshot 2020-05-09 23 08 26

Which might further signal a hydration problem with SSR.

@cansin
Copy link

cansin commented May 9, 2020

Huh, this is not happening on actual production, only happens when I run:

cansin@localhost web % export NODE_ENV=production
cansin@localhost web % yarn next build
cansin@localhost web % yarn next start

locally. So perhaps it is a false alarm? If so, I wonder what is the right way to simulate production locally.

@cansin
Copy link

cansin commented May 9, 2020

Ok, I changed ports and back and this problem disappeared. Perhaps some kind of stale module hot-reload shenanigan was happening, despite trying to run a prod build. Sorry for the misdirection.

@Timer
Copy link
Member

Timer commented Sep 12, 2020

Closing this issue as the issue no longer reproduces on nextjs.org and these experimental features have been enabled by default for a while (minus modern), with no further reports.

@Timer Timer closed this as completed Sep 12, 2020
@andrei-svistunou
Copy link

Hi @Timer ,

I still can reproduce this issue on a lot of sites, including nextjs.org
image

Could you, please, clarify this behaviour? Is it expected?

@andrei-svistunou
Copy link

Hi @Timer
Could you please take a look at my comment above? May we reopen the ticket or better to open a new one?

@helfi92
Copy link

helfi92 commented Feb 8, 2021

We're seeing the same behavior as well on the latest version of Next.js 10.0.6

@SidOfc
Copy link

SidOfc commented Mar 5, 2021

This seems to be happening in Chrome, or perhaps Chromium based browsers in general (didn't test that) in newer versions of next.js. In Firefox it seems like no request is duplicated. However, older versions of next.js don't seem to suffer from this issue even in Chrome.

For reference I'm testing this on Ubuntu 20.04 using Chrome 88.0.4324.182 and Firefox 86.0. Just specifying this in case it isn't reproducible on macOS / Windows, in which case it might be an issue with Chrome on Ubuntu.

Below some information with screenshots taken in Chrome.

I'm currently running next@9.5.5 to prevent this issue (see edit at the bottom). The repository for my website is public in case anyone is interested in running it locally. To reproduce the issue, simply change the version of "next" in package.json to "^10.0.7". Both requests shown here seem to be fired through the commons chunk in next.js 10+ if I am to believe the network inspector:

image

Additionally I did the same thing @andrei-svistunou did and can confirm that nextjs.org has this issue:

image

I've also tried setting the experimental options mentioned at the start of this issue to see if that would resolve it, I tried:

{
   experimental: {
        granularChunks: false,
        css: false,
        modern: false,
    }
}

but alas, no luck here either.

A friend and colleague of mine also has a next.js website using next@9.3.4, this does not show any duplicated requests. This also has a public repo for those interested.

Let me know if you want me to try anything to see if it resolves the issue and last but certainly not least, thanks for building such an awesome framework, I'm loving every bit of it!

EDIT

I've downgraded my own website to next@9.5.5 locally and the issue seems to have disappeared. My website went from loading ~500kb worth of resources to ~170kb on initial page load.

@Sheraff
Copy link
Contributor

Sheraff commented Mar 5, 2021

I was looking for the common denominator of all "duplicate" network calls that actually got through (weren't caught by the cache) and it seems that i found it: the first request is always coming from (index)
Screen Shot 2021-03-05 at 23 49 47

This means that the behavior we're seeing comes from the <link rel="preload" that are initially found in the HTML and not the <link rel="prefetch" added by the script afterwards.

The second request (the duplicate one) always comes from the same piece of code. It's the last line in the following function, packages/next/client/route-loader.ts, lines 91 - 115 (in the /master branch):

function prefetchViaDom(
  href: string,
  as: string,
  link?: HTMLLinkElement
): Promise<any> {
  return new Promise((res, rej) => {
    if (document.querySelector(`link[rel="prefetch"][href^="${href}"]`)) {
      return res()
    }

    link = document.createElement('link')

    // The order of property assignment here is intentional:
    if (as) link!.as = as
    link!.rel = `prefetch`
    link!.crossOrigin = process.env.__NEXT_CROSS_ORIGIN!
    link!.onload = res
    link!.onerror = rej

    // `href` should always be last:
    link!.href = href

    document.head.appendChild(link)
  })
}

My guess is that this function might need to extend its DOM selection to also look for link[rel="preload"] and not just prefetch. But to know for sure, I'd have to test a build with this change and I don't really have the time right now. Anyone who reproduces this issue on one of their current projects is willing to give it a go?

If so, just replace the selector from link[rel="prefetch"][href^="${href}"] to link[rel="prefetch"][href^="${href}"],link[rel="preload"][href^="${href}"].


To clarify, for all "duplicate" script requests that do get caught by the cache, they follow the normal principle of nextjs which is to be added to the DOM as <link rel="prefetch" (by the prefetchViaDom function pasted above) and then actually fetched by adding them to the DOM as <script src="...".

So this leaves me wondering why we don't see three copies of the same request in the case investigated above: 1 preload initially present, 1 prefetch added asap, and 1 regular script whenever needed. So maybe my grasp on the inner workings of nextjs is not very strong and you should take everything I said w/ a grain of salt!

Sheraff added a commit to Sheraff/next.js that referenced this issue Mar 6, 2021
Sheraff added a commit to Sheraff/next.js that referenced this issue Mar 6, 2021
@Sheraff
Copy link
Contributor

Sheraff commented Mar 6, 2021

@SidOfc If you want to test it out, the easiest way would be to search for link[rel="prefetch"][href^="${href}"] inside of your /node_modules/next folder and replace the string with link[rel="prefetch"][href^="${href}"],link[rel="preload"][href^="${href}"].

If you need a solution that lasts a little longer you could always point your package.json to my fork of nextjs instead of the official one, but I really don't recommend that long term (or even just in production for that matter) because it will be unmaintained.

@SidOfc
Copy link

SidOfc commented Mar 6, 2021

@Sheraff alright, I did a search and found one occurrence in dist/client/route-loader.js which I replaced with the string you provided. After running a new build + export however, I'm still seeing duplicate requests:

image

I think the issue lies elsewhere, as I mentioned in my original comment (and can be seen in this screenshot) the requests seem to be coming from the commons chunk and neither of the types is index but rather script or javascript.

I hope this helps further narrowing it down a bit, if there's anything else you'd like me to try feel free to suggest, I think I can make any change necessary in the node_modules/next folder.

EDIT

Also noticing that css requests seem to be duplicated as well, the bottom two entries of my screenshot shows one request with type stylesheet and another with type fetch.

@Sheraff
Copy link
Contributor

Sheraff commented Mar 6, 2021

@SidOfc some duplication is normal and expected w/ nextjs. Some of it will appear from the preload or prefetch tags, and some of it will come from the actual requests that plan on using the resource. You should look at this with cache enabled and look at whether the resource is actually downloaded twice or just requested twice.

To give you an example, from vercel.com, this is what is normal:
Screen Shot 2021-03-06 at 16 13 48

And this is what we're trying to avoid:
Screen Shot 2021-03-06 at 16 14 00

In both cases, one of the requests comes from the prefetchViaDom function inside route-loader.js, that gets bundled into the commons chunk (this is where I asked you to replace the string). But this is just about prefetching the resource. Later on, a resource might be requested again but the request won't go through the network and be caught by the browser's cache which will answer with the prefetched resource (which is why you can't debug this w/ the "disable cache" devtools option toggled on).

@Sheraff
Copy link
Contributor

Sheraff commented Mar 6, 2021

And as for CSS, I had looked into it quickly myself, and couldn't find anywhere where it was an issue, since in all the cases of duplication I could find, the duplicate resource came from the cache. Here's an example of this from nextjs.org:
Screen Shot 2021-03-06 at 16 25 27

@SidOfc
Copy link

SidOfc commented Mar 6, 2021

@Sheraff I see, indeed I did have cache disabled while testing this, thanks for pointing this out. After 1 reload I still noticed some requests were duplicated but after a second reload indeed all requests are correctly retrieved from prefetch/disk cache as seen in your screenshots.

The only thing I might add would be that this works flawlessly when using next start, however after running next export, if I use some other static file server such as serve and run serve out I do see duplicated CSS requests still. The reason for this seems to be that serve does not send appropiate caching headers:

serve
image

next start
image

I'm adding this just to further clarify why some resources might be loading twice in certain scenarios, Next.js has no control over how these static files get served so this is definitely not a Next.js issue.

Thank you for your time, effort, and explanation. It's much appreciated and clears up [my] confusion about how this works. I think I'll be switching back to next@10.0.7 now as that is the way forward :)

EDIT

P.S. is there any way to buy you a crate of beer or something?

@Sheraff
Copy link
Contributor

Sheraff commented Mar 6, 2021

@SidOfc Yeah I haven't tried setting cache headers w/ Next, my current project uses an express server (because it was started before next@10) and all cache headers are written from there.

I'm usually pretty happy w/ magic internet points, but there is always the buy me a coffee thing for special occasions.

@SidOfc
Copy link

SidOfc commented Mar 6, 2021

More than happy to reward the kindness and constructiveness, enjoy 👍

kodiakhq bot pushed a commit that referenced this issue Sep 19, 2021
This PR proposes a fix for #11107 (JS modules are loaded twice). A more detailed explanation of the investigation that led to this PR can be found in the issue's comments (#11107 (comment)).

## Replicability

To identify that the issue replicates on any given project, you need to 
1. look at the network tab (first/clean load of site, so preferably ⌘+⇧+R on an incognito tab), 
2. sort by "name", and filter requests by `mime-type:application/javascript` (selecting "JS" in the devtools filters will actually show all "script" types, but ignore all "javascript" types)
3. look for pairs of identical calls with one originating from initial HTML (`preload` of priority "high" originating from "(index)" or "([page name])")  and another one from a script (`prefetch` of priority "lowest" originating from a .js file), where neither of the files is served from the cache.

Here's a screenshot of an example of what to look for:
<img width="601" alt="Screen Shot 2021-03-07 at 09 59 18" src="https://user-images.githubusercontent.com/1325721/110234627-cf1c6d00-7f2b-11eb-9cd7-749bf881ba56.png">


The issue was reproduced easily on the following projects:
- On [nextjs.org](https://nextjs.org/) where duplicates add up to ~70kB of transferred javascript out of 470kB (14.9%).
- On [vercel.com](https://vercel.com/) where duplicates add up to ~105kB of transferred javascript out of 557kB (18.8%).
- On [tiktok.com](https://tiktok.com/en) where duplicates add up to ~514kB of transferred javascript out of 1556kB (33%).
- In my own project using `"next": "^10.0.1"` (private repo) where duplicates add up to about 5% of total transferred javascript.
- In the issue's comments, a developer reported a replication using `"^10.0.7"` on a [public repo](https://github.com/SidOfc/sidneyliebrand.io).


## Some information about the fix

- Both `preload` and `prefetch` values for `<link rel="x">` behave similarly, with the difference being in network priority level (preload is high priority, prefetch is lowest priority).

- Next.js uses `<link rel="preload">` in its initial HTML but then *only* uses `<link rel="prefetch">` for the rest of the lifetime of the page. 

- However, when Next.js detects that a script should be requested in advance, it only checks for matching `<link rel="prefetch">` and not `<link rel="preload">` (which have higher priority and are present earlier in the DOM, thus have a greater likelihood of being already loaded). 

This PR aims to fix that oversight.

## Potential issues (none AFAIK)

As far as I can tell by looking through the codebase, **there is no downside** not to add a `prefetch` when a `preload` is already in the DOM. No other script looks for a `<link>` based on its `rel` attribute.
@balazsorban44
Copy link
Member

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.

@vercel vercel locked as resolved and limited conversation to collaborators Jan 28, 2022
natew pushed a commit to natew/next.js that referenced this issue Feb 16, 2022
This PR proposes a fix for vercel#11107 (JS modules are loaded twice). A more detailed explanation of the investigation that led to this PR can be found in the issue's comments (vercel#11107 (comment)).

## Replicability

To identify that the issue replicates on any given project, you need to 
1. look at the network tab (first/clean load of site, so preferably ⌘+⇧+R on an incognito tab), 
2. sort by "name", and filter requests by `mime-type:application/javascript` (selecting "JS" in the devtools filters will actually show all "script" types, but ignore all "javascript" types)
3. look for pairs of identical calls with one originating from initial HTML (`preload` of priority "high" originating from "(index)" or "([page name])")  and another one from a script (`prefetch` of priority "lowest" originating from a .js file), where neither of the files is served from the cache.

Here's a screenshot of an example of what to look for:
<img width="601" alt="Screen Shot 2021-03-07 at 09 59 18" src="https://user-images.githubusercontent.com/1325721/110234627-cf1c6d00-7f2b-11eb-9cd7-749bf881ba56.png">


The issue was reproduced easily on the following projects:
- On [nextjs.org](https://nextjs.org/) where duplicates add up to ~70kB of transferred javascript out of 470kB (14.9%).
- On [vercel.com](https://vercel.com/) where duplicates add up to ~105kB of transferred javascript out of 557kB (18.8%).
- On [tiktok.com](https://tiktok.com/en) where duplicates add up to ~514kB of transferred javascript out of 1556kB (33%).
- In my own project using `"next": "^10.0.1"` (private repo) where duplicates add up to about 5% of total transferred javascript.
- In the issue's comments, a developer reported a replication using `"^10.0.7"` on a [public repo](https://github.com/SidOfc/sidneyliebrand.io).


## Some information about the fix

- Both `preload` and `prefetch` values for `<link rel="x">` behave similarly, with the difference being in network priority level (preload is high priority, prefetch is lowest priority).

- Next.js uses `<link rel="preload">` in its initial HTML but then *only* uses `<link rel="prefetch">` for the rest of the lifetime of the page. 

- However, when Next.js detects that a script should be requested in advance, it only checks for matching `<link rel="prefetch">` and not `<link rel="preload">` (which have higher priority and are present earlier in the DOM, thus have a greater likelihood of being already loaded). 

This PR aims to fix that oversight.

## Potential issues (none AFAIK)

As far as I can tell by looking through the codebase, **there is no downside** not to add a `prefetch` when a `preload` is already in the DOM. No other script looks for a `<link>` based on its `rel` attribute.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants