Include submodules in exported type definition #28316

lfades · 2021-08-20T06:53:29Z

Currently, it's not possible to autocomplete imports coming from submodules in the Next.js package, like next/image, next/script, next/router, etc.

With this PR there's a new index.d.ts that adds a reference to most of the submodules, excluding babel, client, constants and data (which points to a non-existing file so maybe it should be removed?). Adding them to the current types/index.d.ts results in an error because these references are only valid after dist has the definitions inside.

Adding tests here is not possible I believe, there are no real changes more than improved autocompletion

Note: It's possible to also get rid of next-env.d.ts for every Next.js app if wanted, by moving the references in there to files in tsconfig.json instead of using includes, or by adding a tsconfig in Next.js and then using extends.

lfades · 2021-08-20T06:54:34Z

packages/next/lib/typescript/writeAppTypeDeclarations.ts

-    '/// <reference types="next/types/global" />' +
-    os.EOL +


This one is included by /// <reference types="next" />'

packages/next/index.d.ts

ijjk · 2021-10-22T04:31:37Z

packages/next/package.json

@@ -7,7 +7,7 @@
  "repository": "vercel/next.js",
  "bugs": "https://github.com/vercel/next.js/issues",
  "homepage": "https://nextjs.org",
-  "types": "types/index.d.ts",
+  "types": "index.d.ts",


This needs to be added to the package.json files array if we're moving it up.

@ijjk It's added on line 45, lmk if you're referring to something else

Is there any benefit to moving this file? This will now be ignored while type-checking the project when it previously wouldn't be.

@ijjk The current issue with types/index.d.ts is that it's included in TS compilation, the references added in index.d.ts will make TS fail when it's trying to build the files it's referencing, therefore by not importing index.d.ts anywhere it's possible to still "export" references

ijjk

Tested this change locally and it does appear to provide much better autocomplete, could you resolve the conflicts with canary?

…ude-submodules

ijjk · 2021-11-06T18:00:01Z

Failing test suites

Commit: f6876f4

test/unit/write-app-declarations.test.ts

find config > should preserve CRLF EOL
find config > should preserve LF EOL
find config > should use OS EOL by default

Expand output

● find config › should preserve CRLF EOL

expect(received).toBe(expected) // Object.is equality

- Expected  - 2
+ Received  + 1

- /// <reference types="next" />
- /// <reference types="next/types/global" />
+ /// <reference types="next" />
  
  // NOTE: This file should not be edited
  // see https://nextjs.org/docs/basic-features/typescript for more information.
  ↵

  32 |
  33 |     await writeAppTypeDeclarations(fixtureDir, imageImportsEnabled)
> 34 |     expect(await fs.readFile(declarationFile, 'utf8')).toBe(content)
     |                                                        ^
  35 |   })
  36 |
  37 |   it('should preserve LF EOL', async () => {

  at Object.<anonymous> (unit/write-app-declarations.test.ts:34:56)

● find config › should preserve LF EOL

expect(received).toBe(expected) // Object.is equality

- Expected  - 2
+ Received  + 1

- /// <reference types="next" />
- /// <reference types="next/types/global" />
+ /// <reference types="next" />

  // NOTE: This file should not be edited
  // see https://nextjs.org/docs/basic-features/typescript for more information.
  ↵

  55 |
  56 |     await writeAppTypeDeclarations(fixtureDir, imageImportsEnabled)
> 57 |     expect(await fs.readFile(declarationFile, 'utf8')).toBe(content)
     |                                                        ^
  58 |   })
  59 |
  60 |   it('should use OS EOL by default', async () => {

  at Object.<anonymous> (unit/write-app-declarations.test.ts:57:56)

● find config › should use OS EOL by default

expect(received).toBe(expected) // Object.is equality

- Expected  - 2
+ Received  + 1

- /// <reference types="next" />
- /// <reference types="next/types/global" />
+ /// <reference types="next" />

  // NOTE: This file should not be edited
  // see https://nextjs.org/docs/basic-features/typescript for more information.
  ↵

  76 |     await fs.ensureDir(fixtureDir)
  77 |     await writeAppTypeDeclarations(fixtureDir, imageImportsEnabled)
> 78 |     expect(await fs.readFile(declarationFile, 'utf8')).toBe(content)
     |                                                        ^
  79 |   })
  80 | })
  81 |

  at Object.<anonymous> (unit/write-app-declarations.test.ts:78:56)

lfades · 2021-11-07T02:58:23Z

There's a lint error in test/lib/next-modes/next-dev.ts unrelated to this PR

styfle · 2021-11-08T17:27:10Z

@lfades The "find config" tests need to be updated in test/unit/write-app-declarations.test.ts

https://github.com/vercel/next.js/runs/4126934126?check_suite_focus=true#step:4:183

…clude-submodules

lfades · 2021-11-24T20:33:37Z

@styfle Updated!

styfle · 2021-11-24T20:35:42Z

packages/create-next-app/templates/typescript/next-env.d.ts

@@ -1,5 +1,4 @@
 /// <reference types="next" />
-/// <reference types="next/types/global" />
 /// <reference types="next/image-types/global" />


Should next/image-types/global be moved too?

That one is under a conditional so I decided not to include it because of that.

styfle · 2021-11-24T20:37:43Z

packages/next/lib/typescript/writeAppTypeDeclarations.ts

@@ -30,8 +30,6 @@ export async function writeAppTypeDeclarations(
  const content =
    '/// <reference types="next" />' +
    eol +
-    '/// <reference types="next/types/global" />' +
-    eol +
    (imageImportsEnabled
      ? '/// <reference types="next/image-types/global" />' + eol


What about next/image-types/global. Can we make that dynamic in the index.d.ts file?

…clude-submodules

ijjk · 2021-11-25T02:58:09Z

Stats from current PR

Default Build (Decrease detected ✓)

General Overall increase ⚠️

	vercel/next.js canary	lfades/next.js ts-include-submodules	Change
buildDuration	22.3s	23.5s	⚠️ +1.2s
buildDurationCached	4.2s	4.5s	⚠️ +297ms
nodeModulesSize	345 MB	345 MB	⚠️ +494 B

Page Load Tests Overall decrease ⚠️

	vercel/next.js canary	lfades/next.js ts-include-submodules	Change
/ failed reqs	0	0	✓
/ total time (seconds)	3.902	3.86	-0.04
/ avg req/sec	640.76	647.75	+6.99
/error-in-render failed reqs	0	0	✓
/error-in-render total time (seconds)	2.053	2.156	⚠️ +0.1
/error-in-render avg req/sec	1217.64	1159.8	⚠️ -57.84

Client Bundles (main, webpack, commons)

	vercel/next.js canary	lfades/next.js ts-include-submodules	Change
450.HASH.js gzip	179 B	179 B	✓
framework-HASH.js gzip	42.2 kB	42.2 kB	✓
main-HASH.js gzip	28.4 kB	28.4 kB	✓
webpack-HASH.js gzip	1.45 kB	1.45 kB	✓
Overall change	72.2 kB	72.2 kB	✓

Legacy Client Bundles (polyfills)

	vercel/next.js canary	lfades/next.js ts-include-submodules	Change
polyfills-HASH.js gzip	31 kB	31 kB	✓
Overall change	31 kB	31 kB	✓

Client Pages

	vercel/next.js canary	lfades/next.js ts-include-submodules	Change
_app-HASH.js gzip	1.37 kB	1.37 kB	✓
_error-HASH.js gzip	194 B	194 B	✓
amp-HASH.js gzip	312 B	312 B	✓
css-HASH.js gzip	327 B	327 B	✓
dynamic-HASH.js gzip	2.38 kB	2.38 kB	✓
head-HASH.js gzip	350 B	350 B	✓
hooks-HASH.js gzip	635 B	635 B	✓
image-HASH.js gzip	4.45 kB	4.45 kB	✓
index-HASH.js gzip	263 B	263 B	✓
link-HASH.js gzip	1.87 kB	1.87 kB	✓
routerDirect..HASH.js gzip	321 B	321 B	✓
script-HASH.js gzip	383 B	383 B	✓
withRouter-HASH.js gzip	318 B	318 B	✓
85e02e95b279..7e3.css gzip	107 B	107 B	✓
Overall change	13.3 kB	13.3 kB	✓

Client Build Manifests

	vercel/next.js canary	lfades/next.js ts-include-submodules	Change
_buildManifest.js gzip	460 B	460 B	✓
Overall change	460 B	460 B	✓

Rendered Page Sizes

	vercel/next.js canary	lfades/next.js ts-include-submodules	Change
index.html gzip	531 B	531 B	✓
link.html gzip	545 B	545 B	✓
withRouter.html gzip	525 B	525 B	✓
Overall change	1.6 kB	1.6 kB	✓

Default Build with SWC (Increase detected ⚠️)

General Overall increase ⚠️

	vercel/next.js canary	lfades/next.js ts-include-submodules	Change
buildDuration	24.2s	24.4s	⚠️ +152ms
buildDurationCached	4.3s	4.2s	-95ms
nodeModulesSize	345 MB	345 MB	⚠️ +494 B

Page Load Tests Overall increase ✓

	vercel/next.js canary	lfades/next.js ts-include-submodules	Change
/ failed reqs	0	0	✓
/ total time (seconds)	3.996	3.91	-0.09
/ avg req/sec	625.64	639.37	+13.73
/error-in-render failed reqs	0	0	✓
/error-in-render total time (seconds)	2.06	2.029	-0.03
/error-in-render avg req/sec	1213.53	1231.86	+18.33

Client Bundles (main, webpack, commons)

	vercel/next.js canary	lfades/next.js ts-include-submodules	Change
450.HASH.js gzip	179 B	179 B	✓
framework-HASH.js gzip	42.3 kB	42.3 kB	✓
main-HASH.js gzip	28.6 kB	28.6 kB	✓
webpack-HASH.js gzip	1.44 kB	1.44 kB	✓
Overall change	72.5 kB	72.5 kB	✓

Legacy Client Bundles (polyfills)

	vercel/next.js canary	lfades/next.js ts-include-submodules	Change
polyfills-HASH.js gzip	31 kB	31 kB	✓
Overall change	31 kB	31 kB	✓

Client Pages

	vercel/next.js canary	lfades/next.js ts-include-submodules	Change
_app-HASH.js gzip	1.35 kB	1.35 kB	✓
_error-HASH.js gzip	180 B	180 B	✓
amp-HASH.js gzip	305 B	305 B	✓
css-HASH.js gzip	321 B	321 B	✓
dynamic-HASH.js gzip	2.37 kB	2.37 kB	✓
head-HASH.js gzip	342 B	342 B	✓
hooks-HASH.js gzip	622 B	622 B	✓
image-HASH.js gzip	4.47 kB	4.47 kB	✓
index-HASH.js gzip	256 B	256 B	✓
link-HASH.js gzip	1.91 kB	1.91 kB	✓
routerDirect..HASH.js gzip	314 B	314 B	✓
script-HASH.js gzip	375 B	375 B	✓
withRouter-HASH.js gzip	309 B	309 B	✓
85e02e95b279..7e3.css gzip	107 B	107 B	✓
Overall change	13.2 kB	13.2 kB	✓

Client Build Manifests

	vercel/next.js canary	lfades/next.js ts-include-submodules	Change
_buildManifest.js gzip	458 B	458 B	✓
Overall change	458 B	458 B	✓

Rendered Page Sizes

	vercel/next.js canary	lfades/next.js ts-include-submodules	Change
index.html gzip	533 B	533 B	✓
link.html gzip	546 B	546 B	✓
withRouter.html gzip	527 B	527 B	✓
Overall change	1.61 kB	1.61 kB	✓

Commit: 98d9ed8

Added a new index file

e22d218

lfades requested review from huozhi, ijjk, padmaia, shuding, styfle and timneutkens as code owners August 20, 2021 06:53

ijjk added create-next-app Related to our CLI tool for quickly starting a new Next.js application. type: next labels Aug 20, 2021

lfades commented Aug 20, 2021

View reviewed changes

packages/next/index.d.ts Outdated Show resolved Hide resolved

This comment has been minimized.

Sign in to view

Add it to "files" just in case

54efdb9

This comment has been minimized.

Sign in to view

ijjk reviewed Oct 22, 2021

View reviewed changes

styfle previously approved these changes Oct 23, 2021

View reviewed changes

Merge branch canary of https://github.com/vercel/next.js into ts-incl…

4a509f9

…ude-submodules

lfades dismissed styfle’s stale review via 4a509f9 November 6, 2021 15:45

Added server.d.ts

8b8c321

lfades requested a review from ijjk November 6, 2021 15:52

This comment has been minimized.

Sign in to view

Added default export

f6876f4

This comment has been minimized.

Sign in to view

lfades added 2 commits November 24, 2021 12:02

Merge branch 'canary' of https://github.com/vercel/next.js into ts-in…

30ee48d

…clude-submodules

Updated "find config" tests

e05b66d

styfle previously approved these changes Nov 24, 2021

View reviewed changes

styfle reviewed Nov 24, 2021

View reviewed changes

This comment has been minimized.

Sign in to view

Updated examples

fdff6e3

lfades dismissed styfle’s stale review via fdff6e3 November 24, 2021 21:36

lfades requested a review from leerob as a code owner November 24, 2021 21:36

ijjk added the examples Issue was opened via the examples template. label Nov 24, 2021

styfle previously approved these changes Nov 24, 2021

View reviewed changes

This comment has been minimized.

Sign in to view

lfades added 2 commits November 24, 2021 18:34

Merge branch 'canary' of https://github.com/vercel/next.js into ts-in…

d6eb0e1

…clude-submodules

Type fix

19d85d3

lfades dismissed styfle’s stale review via 19d85d3 November 25, 2021 00:58

This comment has been minimized.

Sign in to view

styfle previously approved these changes Nov 25, 2021

View reviewed changes

Fixed more NODE_ENV cases

98d9ed8

lfades dismissed styfle’s stale review via 98d9ed8 November 25, 2021 02:32

styfle approved these changes Nov 25, 2021

View reviewed changes

timneutkens merged commit 77150da into vercel:canary Nov 26, 2021

timneutkens mentioned this pull request Dec 2, 2021

Visual Studio Code - IntelliSense for JS and TS not working properly with NextJs. #32045

Closed

puku0x mentioned this pull request Dec 8, 2021

feat(nextjs): bump Next.js version to 12.0.7 nrwl/nx#7788

Merged

balazsorban44 mentioned this pull request Dec 24, 2021

Cannot find name 'StaticImageData'. #29788

Closed

puku0x mentioned this pull request Dec 25, 2021

cleanup(nextjs): remove unnecessary type in next-env.d.ts nrwl/nx#8301

Merged

vercel locked as resolved and limited conversation to collaborators Jan 27, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Include submodules in exported type definition #28316

Include submodules in exported type definition #28316

lfades commented Aug 20, 2021 •

edited

Loading

lfades Aug 20, 2021

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

ijjk Oct 22, 2021

lfades Nov 6, 2021

ijjk Nov 8, 2021

lfades Nov 8, 2021

ijjk left a comment

This comment has been minimized.

This comment has been minimized.

ijjk commented Nov 6, 2021

This comment has been minimized.

lfades commented Nov 7, 2021

styfle commented Nov 8, 2021

lfades commented Nov 24, 2021

styfle Nov 24, 2021 •

edited

Loading

lfades Nov 24, 2021

styfle Nov 24, 2021

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

ijjk commented Nov 25, 2021

Include submodules in exported type definition #28316

Include submodules in exported type definition #28316

Conversation

lfades commented Aug 20, 2021 • edited Loading

lfades Aug 20, 2021

Choose a reason for hiding this comment

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

ijjk Oct 22, 2021

Choose a reason for hiding this comment

lfades Nov 6, 2021

Choose a reason for hiding this comment

ijjk Nov 8, 2021

Choose a reason for hiding this comment

lfades Nov 8, 2021

Choose a reason for hiding this comment

ijjk left a comment

Choose a reason for hiding this comment

This comment has been minimized.

This comment has been minimized.

ijjk commented Nov 6, 2021

Failing test suites

This comment has been minimized.

lfades commented Nov 7, 2021

styfle commented Nov 8, 2021

lfades commented Nov 24, 2021

styfle Nov 24, 2021 • edited Loading

Choose a reason for hiding this comment

lfades Nov 24, 2021

Choose a reason for hiding this comment

styfle Nov 24, 2021

Choose a reason for hiding this comment

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

ijjk commented Nov 25, 2021

Stats from current PR

lfades commented Aug 20, 2021 •

edited

Loading

styfle Nov 24, 2021 •

edited

Loading