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

Fix: Throw an error for empty array return in generateStaticParams with output:export #57053

Merged
merged 9 commits into from
Jan 4, 2024

Conversation

k-taro56
Copy link
Contributor

Fixes #57038

What?

Added an error message when generateStaticParams returns an empty array with output:export.

Why?

To provide developers with clear feedback when generateStaticParams is not used correctly.

How?

Modified the condition checks around the use of generateStaticParams to include a check for an empty array and added a corresponding error message.

for error handling when generateStaticParams returns an empty array
@dobrakmato
Copy link

Hey @timneutkens is there anything that needs to be done, before this can be merged? The error message is currently confusing. Also do you accept PRs implementing this functionality?

@ijjk
Copy link
Member

ijjk commented Jan 2, 2024

Tests Passed

@ijjk
Copy link
Member

ijjk commented Jan 2, 2024

Stats from current PR

Default Build (Increase detected ⚠️)
General Overall increase ⚠️
vercel/next.js canary k-taro56/next.js fix-empty-gsp-return Change
buildDuration 12.7s 12.8s ⚠️ +126ms
buildDurationCached 7.1s 6.1s N/A
nodeModulesSize 200 MB 202 MB ⚠️ +2.14 MB
nextStartRea..uration (ms) 421ms 501ms N/A
Client Bundles (main, webpack)
vercel/next.js canary k-taro56/next.js fix-empty-gsp-return Change
193.HASH.js gzip 181 B 182 B N/A
3f784ff6-HASH.js gzip 53.3 kB 53.3 kB
433-HASH.js gzip 28.5 kB 28.5 kB N/A
framework-HASH.js gzip 45.2 kB 45.2 kB
main-app-HASH.js gzip 239 B 242 B N/A
main-HASH.js gzip 31.7 kB 31.7 kB N/A
webpack-HASH.js gzip 1.7 kB 1.7 kB N/A
Overall change 98.5 kB 98.5 kB
Legacy Client Bundles (polyfills)
vercel/next.js canary k-taro56/next.js fix-empty-gsp-return Change
polyfills-HASH.js gzip 31 kB 31 kB
Overall change 31 kB 31 kB
Client Pages
vercel/next.js canary k-taro56/next.js fix-empty-gsp-return Change
_app-HASH.js gzip 194 B 195 B N/A
_error-HASH.js gzip 183 B 181 B N/A
amp-HASH.js gzip 504 B 502 B N/A
css-HASH.js gzip 321 B 321 B
dynamic-HASH.js gzip 2.5 kB 2.5 kB N/A
edge-ssr-HASH.js gzip 255 B 253 B N/A
head-HASH.js gzip 350 B 349 B N/A
hooks-HASH.js gzip 369 B 369 B
image-HASH.js gzip 4.28 kB 4.28 kB N/A
index-HASH.js gzip 255 B 256 B N/A
link-HASH.js gzip 2.61 kB 2.61 kB
routerDirect..HASH.js gzip 312 B 311 B N/A
script-HASH.js gzip 385 B 383 B N/A
withRouter-HASH.js gzip 307 B 308 B N/A
1afbb74e6ecf..834.css gzip 106 B 106 B
Overall change 3.4 kB 3.4 kB
Client Build Manifests
vercel/next.js canary k-taro56/next.js fix-empty-gsp-return Change
_buildManifest.js gzip 483 B 484 B N/A
Overall change 0 B 0 B
Rendered Page Sizes
vercel/next.js canary k-taro56/next.js fix-empty-gsp-return Change
index.html gzip 526 B 526 B
link.html gzip 539 B 538 B N/A
withRouter.html gzip 523 B 521 B N/A
Overall change 526 B 526 B
Edge SSR bundle Size
vercel/next.js canary k-taro56/next.js fix-empty-gsp-return Change
edge-ssr.js gzip 93.8 kB 93.8 kB N/A
page.js gzip 147 kB 147 kB N/A
Overall change 0 B 0 B
Middleware size
vercel/next.js canary k-taro56/next.js fix-empty-gsp-return Change
middleware-b..fest.js gzip 626 B 623 B N/A
middleware-r..fest.js gzip 151 B 151 B
middleware.js gzip 37.4 kB 37.4 kB N/A
edge-runtime..pack.js gzip 1.92 kB 1.92 kB
Overall change 2.07 kB 2.07 kB
Next Runtimes
vercel/next.js canary k-taro56/next.js fix-empty-gsp-return Change
app-page-exp...dev.js gzip 169 kB 168 kB N/A
app-page-exp..prod.js gzip 94.2 kB 94.2 kB N/A
app-page-tur..prod.js gzip 94.9 kB 94.9 kB N/A
app-page-tur..prod.js gzip 89.5 kB 89.4 kB N/A
app-page.run...dev.js gzip 139 kB 138 kB N/A
app-page.run..prod.js gzip 88.8 kB 88.7 kB N/A
app-route-ex...dev.js gzip 24.1 kB 24.1 kB N/A
app-route-ex..prod.js gzip 16.7 kB 16.7 kB N/A
app-route-tu..prod.js gzip 16.7 kB 16.7 kB N/A
app-route-tu..prod.js gzip 16.3 kB 16.3 kB N/A
app-route.ru...dev.js gzip 23.5 kB 23.5 kB N/A
app-route.ru..prod.js gzip 16.3 kB 16.3 kB N/A
pages-api-tu..prod.js gzip 9.38 kB 9.38 kB
pages-api.ru...dev.js gzip 9.65 kB 9.65 kB
pages-api.ru..prod.js gzip 9.37 kB 9.37 kB
pages-turbo...prod.js gzip 21.9 kB 21.9 kB N/A
pages.runtim...dev.js gzip 22.6 kB 22.5 kB N/A
pages.runtim..prod.js gzip 21.9 kB 21.9 kB N/A
server.runti..prod.js gzip 49.4 kB 49.5 kB N/A
Overall change 28.4 kB 28.4 kB
Diff details
Diff for page.js

Diff too large to display

Diff for edge-ssr.js

Diff too large to display

Diff for 433-HASH.js

Diff too large to display

Diff for main-HASH.js

Diff too large to display

Diff for app-page-exp..ntime.dev.js
failed to diff
Diff for app-page-exp..time.prod.js

Diff too large to display

Diff for app-page-tur..time.prod.js

Diff too large to display

Diff for app-page-tur..time.prod.js

Diff too large to display

Diff for app-page.runtime.dev.js
failed to diff
Diff for app-page.runtime.prod.js

Diff too large to display

Diff for app-route-ex..ntime.dev.js

Diff too large to display

Diff for app-route-ex..time.prod.js

Diff too large to display

Diff for app-route-tu..time.prod.js

Diff too large to display

Diff for app-route-tu..time.prod.js

Diff too large to display

Diff for app-route.runtime.dev.js

Diff too large to display

Diff for app-route.ru..time.prod.js

Diff too large to display

Diff for pages-turbo...time.prod.js

Diff too large to display

Diff for pages.runtime.dev.js

Diff too large to display

Diff for pages.runtime.prod.js

Diff too large to display

Diff for server.runtime.prod.js

Diff too large to display

Commit: 47fcdd0

@styfle styfle added the CI approved Approve running CI for fork label Jan 4, 2024
Copy link
Member

@styfle styfle left a comment

Choose a reason for hiding this comment

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

Thanks!

I pushed a change to fix it in next dev too

@styfle styfle merged commit 91dba7d into vercel:canary Jan 4, 2024
68 checks passed
agustints pushed a commit to agustints/next.js that referenced this pull request Jan 6, 2024
…with `output:export` (vercel#57053)

Fixes vercel#57038

# What?
Added an error message when `generateStaticParams` returns an empty
array with `output:export`.

# Why?
To provide developers with clear feedback when `generateStaticParams` is
not used correctly.

# How?
Modified the condition checks around the use of `generateStaticParams`
to include a check for an empty array and added a corresponding error
message.

---------

Co-authored-by: Steven <[email protected]>
@barroudjo
Copy link

I think it's not a good idea to throw an error when generateStaticParams returns an empty array with output: export, maybe a warning, but it shouldn't crash.
For example when working with a CMS with a preview mode, we want to be able to preview the page in SSR, and build the site with output: export when that same page generateStaticParams returns an empty array (because the documents accessed in the page are not published yet, but are available in the preview mode).

@styfle
Copy link
Member

styfle commented Jan 18, 2024

@barroudjo Good point, the existing behavior doesn't account for dynamic data from a CMS.

I'll revert in PR #60831.

Instead of erroring, we should warn (and only in dev mode). Anyone want to create a new PR?

huozhi pushed a commit that referenced this pull request Jan 18, 2024
…Params` with `output:export`" (#60831)

Reverts #57053 per this comment:
#57053 (comment)

Instead of erroring, we should warn (and only in dev mode). That can be
added in a future PR.

Closes NEXT-2155
@github-actions github-actions bot added the locked label Feb 2, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 2, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
CI approved Approve running CI for fork locked type: next
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Error with generateStaticParams function despite its existence
5 participants