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

getStaticProps has about 2-3 seconds of redundant overhead during development #38757

Closed
1 task done
paales opened this issue Jul 18, 2022 · 5 comments · Fixed by #46940
Closed
1 task done

getStaticProps has about 2-3 seconds of redundant overhead during development #38757

paales opened this issue Jul 18, 2022 · 5 comments · Fixed by #46940
Labels
bug Issue was opened via the bug report template.

Comments

@paales
Copy link

paales commented Jul 18, 2022

Verify canary release

  • I verified that the issue exists in the latest Next.js canary release

Provide environment information

Operating System:
  Platform: darwin
  Arch: x64
  Version: Darwin Kernel Version 21.5.0: Tue Apr 26 21:08:22 PDT 2022; root:xnu-8020.121.3~4/RELEASE_X86_64
Binaries:
  Node: 16.15.0
  npm: 8.5.5
  Yarn: 1.19.1
  pnpm: 7.1.5
Relevant packages:
  next: 12.2.2
  eslint-config-next: N/A
  react: 18.2.0
  react-dom: 18.2.0

warn - Latest canary version not detected, detected: "12.2.2", newest: "12.2.3-canary.12".
Please try the latest canary version (npm install next@canary) to confirm the issue still exists before creating a new issue.
Read more - https://nextjs.org/docs/messages/opening-an-issue

What browser are you using? (if relevant)

Not relevant

How are you deploying your application? (if relevant)

Not relvant

Describe the Bug

getStaticPaths+getStaticProps is about 2-3 seconds slower in comparison with getServerSideProps.

The reason this is slower is because next is using a worker to get the getStaticPaths information and the worker isn't reused.

This one is causing about 1.5 seconds slowdown: #11347
Even reusing the worker seems slow as well.

If I trace everything back to the original PR that introduced the worker there is a collapsed comment by @ijjk:

I thought we decided making this blocking would be not ideal as getStaticPaths may have expensive fetching so we wanted to do it lazily to not block the user but still catch any errors in getStaticPaths?

Can this perhaps be revisited as this really slows down development, seems like an easy 2-3 seconds win.

Expected Behavior

getStaticPaths shouldn't be blocking and the actual result probably only is interesting when fallback = false, maybe cache it there?

I think this 'artificial' slowdown here is throwing me off balance what to expect on the production environment in terms of performance. It turned out I didn't program so bad, there was just a 2-3 seconds overhead 😄

Link to reproduction

To Reproduce

use getStaticProps on a page and load during development, you can't get it faster than 2-3 seconds.

@paales paales added the bug Issue was opened via the bug report template. label Jul 18, 2022
@vrushank
Copy link

@paales, we are seeing the same behavior. Do you have any solution or we just need to wait for the team to acknowledge this issue and resolve it ?

@paales
Copy link
Author

paales commented Jan 4, 2023

diff --git a/node_modules/next/dist/server/dev/next-dev-server.js b/node_modules/next/dist/server/dev/next-dev-server.js
index efa36c0..cb6cf1b 100644
--- a/node_modules/next/dist/server/dev/next-dev-server.js
+++ b/node_modules/next/dist/server/dev/next-dev-server.js
@@ -976,6 +976,7 @@ class DevServer extends _nextServer.default {
         const __getStaticPaths = async ()=>{
             const { configFileName , publicRuntimeConfig , serverRuntimeConfig , httpAgentOptions , experimental: { enableUndici  } ,  } = this.nextConfig;
             const { locales , defaultLocale  } = this.nextConfig.i18n || {};
+            return { paths: [], fallback: 'blocking'}
             const pathsResult = await this.getStaticPathsWorker().loadStaticPaths({
                 distDir: this.distDir,
                 pathname,

I forgot to respond until I needed to update the patch (this one is for 13.1.1). With this patch the latency isn't there but seems to only be a workaround. Not sure how to properly fix it.

Maybe someone from the nextjs team can take a look.

@paales
Copy link
Author

paales commented Jan 4, 2023

Something that is easy to fix in nextjs @ijjk?

ijjk added a commit that referenced this issue Mar 8, 2023
Follow-up to #46906 this ensures
we revalidate `generateStaticParams`/`getStaticPaths` in the background
in development so that we aren't blocking refreshes an much
un-necessarily if the paths cache is already populated.

Fixes: #44646
Fixes: #38757
x-ref:
#17977 (comment)
x-ref: #20076
x-ref: #14378
@paales
Copy link
Author

paales commented Mar 9, 2023

Wooo, thanks @ijjk!

@github-actions
Copy link
Contributor

github-actions bot commented Apr 8, 2023

This closed issue has been automatically locked because it had no new activity for a month. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 8, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Issue was opened via the bug report template.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants