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

docs: update get-static-paths.md #40205

Merged
merged 3 commits into from Sep 6, 2022
Merged

docs: update get-static-paths.md #40205

merged 3 commits into from Sep 6, 2022

Conversation

sumiren
Copy link
Contributor

@sumiren sumiren commented Sep 4, 2022

I found that fallback: true behaves like fallback: blocking when client-side page transition, but document doesn't mention the behavior.
I tried following.

  • created a SSG page which has getStaticPaths and getStaticProps, and the getStaticProps is too slow(something like await setTimeout(10000))
  • next build && next start, not next dev
  • tried page reload and client-side page transition both

getServerSideProps document explains about client-side page transition.
https://nextjs.org/docs/basic-features/data-fetching/get-server-side-props#when-does-getserversideprops-run

On the other hand, getStaticPaths didn't explain this behavior, so I added it.

Bug

  • Related issues linked using fixes #number
  • Integration tests added
  • Errors have helpful link attached, see contributing.md

Feature

  • Implements an existing feature request or RFC. Make sure the feature request has been accepted for implementation before opening a PR.
  • Related issues linked using fixes #number
  • Integration tests added
  • Documentation added
  • Telemetry added. In case of a feature if it's used or not.
  • Errors have helpful link attached, see contributing.md

Documentation / Examples

  • Make sure the linting passes by running pnpm lint
  • The examples guidelines are followed from our contributing doc

add explanation when client-side page transition
Copy link
Contributor

@SukkaW SukkaW left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have you checked the source code and implementation details to back your theory?

@sumiren
Copy link
Contributor Author

sumiren commented Sep 4, 2022

@SukkaW
Actually no, this is just based on behavior I observed.
I was not sure whether this behavior is well known in Next.js community or not, so started by fixing documents.

Should I check the source code and identify why it happens?

@SukkaW
Copy link
Contributor

SukkaW commented Sep 4, 2022

@SukkaW Actually no, this is just based on behavior I observed. I was not sure whether this behavior is well known in Next.js community or not, so started by fixing documents.

Should I check the source code and identify why it happens?

You could also use GitHub's blame to find who, when, and where ("where" means in which PR) that this documentation is initially made: It documents this behavior for a reason.

And personally, I didn't notice Next.js has made some changes toward this part recently (thus I believe the behavior may not change).

@sumiren
Copy link
Contributor Author

sumiren commented Sep 5, 2022

@SukkaW

Thanks for your comment!

Maybe I haven't yet understood what you tell me.
I thought that document lacks explanation about certain Next.js behavior , and added some explanation to document.
So, I didn't delete anything.
(I'm sorry if my explanation is not clear.)

You mean that, there has to be a reason that document don't mention client-side page transition, right?

If so, I'll check out git logs related to this page.

Co-authored-by: Lee Robinson <me@leerob.io>
@sumiren
Copy link
Contributor Author

sumiren commented Sep 6, 2022

@leerob
Thanks!
I agreed with your suggestion, and applied it.

Well, do you think this behavior is not a bug but a spec?

When a page with `fallback: true` is navigated to through `next/link` or `next/router` (client-side) Next.js will _not_ serve a fallback and instead the page will behave as `fallback: 'blocking'.

I thought so because you gave me a polish, but I'm a bit worried that this behavior is not intended by Next.js.
If this behavior is a spec or not planned to be changed, I'm glad to see this PR is approved.

Copy link
Member

@ijjk ijjk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, this behavior is expected and sounds good to document, thanks for the PR!

@kodiakhq kodiakhq bot merged commit 5c2faad into vercel:canary Sep 6, 2022
@sumiren
Copy link
Contributor Author

sumiren commented Sep 6, 2022

@ijjk
Okay, I understand.
Thanks!

@sumiren
Copy link
Contributor Author

sumiren commented Sep 6, 2022

@ijjk
If there is, can you tell me issue or discussion link where the behavior was discussed?

behavior is expected

@ijjk
Copy link
Member

ijjk commented Sep 6, 2022

I'm not sure of a specific discussion to link to although could go back to the original RFCs, this has been the expected behavior since getStaticPaths was implemented publicly.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 7, 2022
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 this pull request may close these issues.

None yet

4 participants