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

Update NextConfig type to not require experimental or future fields #25517

Merged
merged 3 commits into from
Jul 20, 2021

Conversation

fabb
Copy link
Contributor

@fabb fabb commented May 27, 2021

when typechecking next.config.js as described in the docs, an error would be thrown if future and experimental are missing. When using Partial<> in the type definition instead, it works as expected.

Fixes #25498

Bug

  • Related issues linked using fixes #number
  • Integration tests added

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.

Documentation / Examples

  • Make sure the linting passes

timneutkens
timneutkens previously approved these changes May 28, 2021
@fabb
Copy link
Contributor Author

fabb commented May 28, 2021

Ok there are quite a lot of build errors because of missing null checks that need to be fixed. Might take a few days until i get to it.

@fabb
Copy link
Contributor Author

fabb commented Jun 6, 2021

Internally next.js spreads the defaultConfig, so internally actually future and experimental cannot become undefined: https://github.com/vercel/next.js/blob/canary/packages/next/next-server/server/config.ts#L115

So maybe it would be better to just suggest Partial<NextConfig> for typechecking next.config.js? I've changed the PR accordingly.

the `experimental` and `future` keys are non-optional in the type NextConfig because internally they are always set to a default from defaultConfig, but in next.config.js they are allowed to be optional
@fabb fabb requested a review from leerob as a code owner June 6, 2021 10:00
@fabb fabb changed the title make all keys of NextConfig optional suggest to use Partial<NextConfig> to typecheck next.config.js Jun 6, 2021
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.

This config has been updated in 9c93c05 so I think we could update the type to mark experimental and future as optional now since we have a separate internal type to avoid the in-accurate errors from them being optional.

@ijjk ijjk changed the title suggest to use Partial<NextConfig> to typecheck next.config.js Update NextConfig type to not require experimental or future fields Jul 20, 2021
@ijjk
Copy link
Member

ijjk commented Jul 20, 2021

Stats from current PR

Default Build (Decrease detected ✓)
General Overall increase ⚠️
vercel/next.js canary fabb/next.js patch-2 Change
buildDuration 16.6s 17.1s ⚠️ +512ms
buildDurationCached 3.8s 3.8s -32ms
nodeModulesSize 49.5 MB 49.5 MB ⚠️ +423 B
Page Load Tests Overall decrease ⚠️
vercel/next.js canary fabb/next.js patch-2 Change
/ failed reqs 0 0
/ total time (seconds) 2.85 2.898 ⚠️ +0.05
/ avg req/sec 877.07 862.66 ⚠️ -14.41
/error-in-render failed reqs 0 0
/error-in-render total time (seconds) 1.784 1.837 ⚠️ +0.05
/error-in-render avg req/sec 1401.67 1360.88 ⚠️ -40.79
Client Bundles (main, webpack, commons)
vercel/next.js canary fabb/next.js patch-2 Change
359.HASH.js gzip 2.96 kB 2.96 kB
745.HASH.js gzip 180 B 180 B
framework-HASH.js gzip 42.2 kB 42.2 kB
main-HASH.js gzip 20.9 kB 20.9 kB
webpack-HASH.js gzip 1.53 kB 1.53 kB
Overall change 67.8 kB 67.8 kB
Legacy Client Bundles (polyfills)
vercel/next.js canary fabb/next.js patch-2 Change
polyfills-HASH.js gzip 31.1 kB 31.1 kB
Overall change 31.1 kB 31.1 kB
Client Pages
vercel/next.js canary fabb/next.js patch-2 Change
_app-HASH.js gzip 803 B 803 B
_error-HASH.js gzip 3.06 kB 3.06 kB
amp-HASH.js gzip 554 B 554 B
css-HASH.js gzip 329 B 329 B
dynamic-HASH.js gzip 2.52 kB 2.52 kB
head-HASH.js gzip 2.28 kB 2.28 kB
hooks-HASH.js gzip 903 B 903 B
image-HASH.js gzip 5.61 kB 5.61 kB
index-HASH.js gzip 261 B 261 B
link-HASH.js gzip 1.66 kB 1.66 kB
routerDirect..HASH.js gzip 319 B 319 B
script-HASH.js gzip 387 B 387 B
withRouter-HASH.js gzip 320 B 320 B
bb14e60e810b..30f.css gzip 125 B 125 B
Overall change 19.1 kB 19.1 kB
Client Build Manifests
vercel/next.js canary fabb/next.js patch-2 Change
_buildManifest.js gzip 490 B 490 B
Overall change 490 B 490 B
Rendered Page Sizes
vercel/next.js canary fabb/next.js patch-2 Change
index.html gzip 531 B 531 B
link.html gzip 544 B 544 B
withRouter.html gzip 525 B 525 B
Overall change 1.6 kB 1.6 kB

Webpack 4 Mode (Increase detected ⚠️)
General Overall increase ⚠️
vercel/next.js canary fabb/next.js patch-2 Change
buildDuration 13.9s 13.1s -805ms
buildDurationCached 5.2s 5.5s ⚠️ +233ms
nodeModulesSize 49.5 MB 49.5 MB ⚠️ +423 B
Page Load Tests Overall increase ✓
vercel/next.js canary fabb/next.js patch-2 Change
/ failed reqs 0 0
/ total time (seconds) 3.027 2.831 -0.2
/ avg req/sec 825.97 882.97 +57
/error-in-render failed reqs 0 0
/error-in-render total time (seconds) 1.943 1.664 -0.28
/error-in-render avg req/sec 1286.67 1502.44 +215.77
Client Bundles (main, webpack, commons)
vercel/next.js canary fabb/next.js patch-2 Change
17.HASH.js gzip 2.98 kB 2.98 kB
18.HASH.js gzip 185 B 185 B
677f882d2ed8..HASH.js gzip 13.7 kB 13.7 kB
framework.HASH.js gzip 41.9 kB 41.9 kB
main-HASH.js gzip 8.4 kB 8.4 kB
webpack-HASH.js gzip 1.22 kB 1.22 kB
Overall change 68.4 kB 68.4 kB
Legacy Client Bundles (polyfills)
vercel/next.js canary fabb/next.js patch-2 Change
polyfills-HASH.js gzip 31.3 kB 31.3 kB
Overall change 31.3 kB 31.3 kB
Client Pages
vercel/next.js canary fabb/next.js patch-2 Change
_app-HASH.js gzip 791 B 791 B
_error-HASH.js gzip 3.76 kB 3.76 kB
amp-HASH.js gzip 552 B 552 B
css-HASH.js gzip 333 B 333 B
dynamic-HASH.js gzip 2.71 kB 2.71 kB
head-HASH.js gzip 2.97 kB 2.97 kB
hooks-HASH.js gzip 911 B 911 B
index-HASH.js gzip 231 B 231 B
link-HASH.js gzip 1.64 kB 1.64 kB
routerDirect..HASH.js gzip 298 B 298 B
script-HASH.js gzip 3.02 kB 3.02 kB
withRouter-HASH.js gzip 294 B 294 B
e025d2764813..52f.css gzip 125 B 125 B
Overall change 17.6 kB 17.6 kB
Client Build Manifests
vercel/next.js canary fabb/next.js patch-2 Change
_buildManifest.js gzip 500 B 500 B
Overall change 500 B 500 B
Rendered Page Sizes
vercel/next.js canary fabb/next.js patch-2 Change
index.html gzip 577 B 577 B
link.html gzip 587 B 587 B
withRouter.html gzip 569 B 569 B
Overall change 1.73 kB 1.73 kB
Commit: 60ea3e0

@kodiakhq kodiakhq bot merged commit 8e34902 into vercel:canary Jul 20, 2021
flybayer pushed a commit to blitz-js/next.js that referenced this pull request Aug 19, 2021
…ercel#25517)

when typechecking next.config.js as described [in the docs](https://github.com/vercel/next.js/blob/canary/docs/basic-features/typescript.md#type-checking-nextconfigjs), an error would be thrown if `future` and `experimental` are missing. When using `Partial<>` in the type definition instead, it works as expected.

Fixes vercel#25498

## Bug

- [ ] Related issues linked using `fixes #number`
- [ ] Integration tests added

## 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.

## Documentation / Examples

- [ ] Make sure the linting passes
@vercel vercel locked as resolved and limited conversation to collaborators Jan 28, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

'future' & 'experimental' keys are set to required in type NextConfig whereas they're supposed to be optional
3 participants