Skip to content

Commit

Permalink
Improve type checking (#41427)
Browse files Browse the repository at this point in the history
This PR implements the process of generating strict "type guard" files,
to ensure that all entrypoints have the correct export types. The type
checking process happens automatically during `next build`.

## Bug

- [ ] Related issues linked using `fixes #number`
- [ ] Integration tests added
- [ ] Errors have a 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 a 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](https://github.com/vercel/next.js/blob/canary/contributing/examples/adding-examples.md)

Co-authored-by: Tobias Koppers <tobias.koppers@googlemail.com>
  • Loading branch information
shuding and sokra committed Oct 19, 2022
1 parent 897af62 commit fda355d
Show file tree
Hide file tree
Showing 18 changed files with 486 additions and 145 deletions.
180 changes: 99 additions & 81 deletions packages/next/build/index.ts
Expand Up @@ -170,7 +170,8 @@ function verifyTypeScriptSetup(
disableStaticImages: boolean,
cacheDir: string | undefined,
numWorkers: number | undefined,
enableWorkerThreads: boolean | undefined
enableWorkerThreads: boolean | undefined,
isAppDirEnabled: boolean
) {
const typeCheckWorker = new JestWorker(
require.resolve('../lib/verifyTypeScriptSetup'),
Expand All @@ -194,6 +195,7 @@ function verifyTypeScriptSetup(
tsconfigPath,
disableStaticImages,
cacheDir,
isAppDirEnabled,
})
.then((result) => {
typeCheckWorker.end()
Expand Down Expand Up @@ -327,100 +329,111 @@ export default async function build(
telemetry.record(events)
)

const ignoreTypeScriptErrors = Boolean(
config.typescript.ignoreBuildErrors
)

const ignoreESLint = Boolean(config.eslint.ignoreDuringBuilds)
const eslintCacheDir = path.join(cacheDir, 'eslint/')
const shouldLint = !ignoreESLint && runLint

if (ignoreTypeScriptErrors) {
Log.info('Skipping validation of types')
}
if (runLint && ignoreESLint) {
// only print log when build requre lint while ignoreESLint is enabled
Log.info('Skipping linting')
}
const startTypeChecking = async () => {
const ignoreTypeScriptErrors = Boolean(
config.typescript.ignoreBuildErrors
)

let typeCheckingAndLintingSpinnerPrefixText: string | undefined
let typeCheckingAndLintingSpinner:
| ReturnType<typeof createSpinner>
| undefined

if (!ignoreTypeScriptErrors && shouldLint) {
typeCheckingAndLintingSpinnerPrefixText =
'Linting and checking validity of types'
} else if (!ignoreTypeScriptErrors) {
typeCheckingAndLintingSpinnerPrefixText = 'Checking validity of types'
} else if (shouldLint) {
typeCheckingAndLintingSpinnerPrefixText = 'Linting'
}
const eslintCacheDir = path.join(cacheDir, 'eslint/')

// we will not create a spinner if both ignoreTypeScriptErrors and ignoreESLint are
// enabled, but we will still verifying project's tsconfig and dependencies.
if (typeCheckingAndLintingSpinnerPrefixText) {
typeCheckingAndLintingSpinner = createSpinner({
prefixText: `${Log.prefixes.info} ${typeCheckingAndLintingSpinnerPrefixText}`,
})
}
if (ignoreTypeScriptErrors) {
Log.info('Skipping validation of types')
}
if (runLint && ignoreESLint) {
// only print log when build requre lint while ignoreESLint is enabled
Log.info('Skipping linting')
}

const typeCheckStart = process.hrtime()
let typeCheckingAndLintingSpinnerPrefixText: string | undefined
let typeCheckingAndLintingSpinner:
| ReturnType<typeof createSpinner>
| undefined

if (!ignoreTypeScriptErrors && shouldLint) {
typeCheckingAndLintingSpinnerPrefixText =
'Linting and checking validity of types'
} else if (!ignoreTypeScriptErrors) {
typeCheckingAndLintingSpinnerPrefixText = 'Checking validity of types'
} else if (shouldLint) {
typeCheckingAndLintingSpinnerPrefixText = 'Linting'
}

try {
const [[verifyResult, typeCheckEnd]] = await Promise.all([
nextBuildSpan.traceChild('verify-typescript-setup').traceAsyncFn(() =>
verifyTypeScriptSetup(
dir,
[pagesDir, appDir].filter(Boolean) as string[],
!ignoreTypeScriptErrors,
config.typescript.tsconfigPath,
config.images.disableStaticImages,
cacheDir,
config.experimental.cpus,
config.experimental.workerThreads
).then((resolved) => {
const checkEnd = process.hrtime(typeCheckStart)
return [resolved, checkEnd] as const
})
),
shouldLint &&
// we will not create a spinner if both ignoreTypeScriptErrors and ignoreESLint are
// enabled, but we will still verifying project's tsconfig and dependencies.
if (typeCheckingAndLintingSpinnerPrefixText) {
typeCheckingAndLintingSpinner = createSpinner({
prefixText: `${Log.prefixes.info} ${typeCheckingAndLintingSpinnerPrefixText}`,
})
}

const typeCheckStart = process.hrtime()

try {
const [[verifyResult, typeCheckEnd]] = await Promise.all([
nextBuildSpan
.traceChild('verify-and-lint')
.traceAsyncFn(async () => {
await verifyAndLint(
.traceChild('verify-typescript-setup')
.traceAsyncFn(() =>
verifyTypeScriptSetup(
dir,
eslintCacheDir,
config.eslint?.dirs,
[pagesDir, appDir].filter(Boolean) as string[],
!ignoreTypeScriptErrors,
config.typescript.tsconfigPath,
config.images.disableStaticImages,
cacheDir,
config.experimental.cpus,
config.experimental.workerThreads,
telemetry,
isAppDirEnabled && !!appDir
)
}),
])
typeCheckingAndLintingSpinner?.stopAndPersist()

if (!ignoreTypeScriptErrors && verifyResult) {
telemetry.record(
eventTypeCheckCompleted({
durationInSeconds: typeCheckEnd[0],
typescriptVersion: verifyResult.version,
inputFilesCount: verifyResult.result?.inputFilesCount,
totalFilesCount: verifyResult.result?.totalFilesCount,
incremental: verifyResult.result?.incremental,
})
)
}
} catch (err) {
// prevent showing jest-worker internal error as it
// isn't helpful for users and clutters output
if (isError(err) && err.message === 'Call retries were exceeded') {
process.exit(1)
isAppDirEnabled
).then((resolved) => {
const checkEnd = process.hrtime(typeCheckStart)
return [resolved, checkEnd] as const
})
),
shouldLint &&
nextBuildSpan
.traceChild('verify-and-lint')
.traceAsyncFn(async () => {
await verifyAndLint(
dir,
eslintCacheDir,
config.eslint?.dirs,
config.experimental.cpus,
config.experimental.workerThreads,
telemetry,
isAppDirEnabled && !!appDir
)
}),
])
typeCheckingAndLintingSpinner?.stopAndPersist()

if (!ignoreTypeScriptErrors && verifyResult) {
telemetry.record(
eventTypeCheckCompleted({
durationInSeconds: typeCheckEnd[0],
typescriptVersion: verifyResult.version,
inputFilesCount: verifyResult.result?.inputFilesCount,
totalFilesCount: verifyResult.result?.totalFilesCount,
incremental: verifyResult.result?.incremental,
})
)
}
} catch (err) {
// prevent showing jest-worker internal error as it
// isn't helpful for users and clutters output
if (isError(err) && err.message === 'Call retries were exceeded') {
process.exit(1)
}
throw err
}
throw err
}

// For app directory, we run type checking after build. That's because
// we dynamically generate types for each layout and page in the app
// directory.
if (!appDir) await startTypeChecking()

const buildLintEvent: EventBuildFeatureUsage = {
featureName: 'build-lint',
invocationCount: shouldLint ? 1 : 0,
Expand Down Expand Up @@ -1038,6 +1051,11 @@ export default async function build(
}
}

// For app directory, we run type checking after build.
if (appDir) {
await startTypeChecking()
}

const postCompileSpinner = createSpinner({
prefixText: `${Log.prefixes.info} Collecting page data`,
})
Expand Down
5 changes: 5 additions & 0 deletions packages/next/build/webpack-config.ts
Expand Up @@ -47,6 +47,7 @@ import { regexLikeCss } from './webpack/config/blocks/css'
import { CopyFilePlugin } from './webpack/plugins/copy-file-plugin'
import { FlightManifestPlugin } from './webpack/plugins/flight-manifest-plugin'
import { FlightClientEntryPlugin } from './webpack/plugins/flight-client-entry-plugin'
import { FlightTypesPlugin } from './webpack/plugins/flight-types-plugin'
import type {
Feature,
SWC_TARGET_TRIPLE,
Expand Down Expand Up @@ -1976,6 +1977,10 @@ export default async function getBaseWebpackConfig(
dev,
isEdgeServer,
})),
hasAppDir &&
!isClient &&
!dev &&
new FlightTypesPlugin({ dir, appDir, dev, isEdgeServer }),
!dev &&
isClient &&
!!config.experimental.sri?.algorithm &&
Expand Down
26 changes: 4 additions & 22 deletions packages/next/build/webpack/plugins/flight-client-entry-plugin.ts
Expand Up @@ -17,9 +17,10 @@ import {
EDGE_RUNTIME_WEBPACK,
FLIGHT_SERVER_CSS_MANIFEST,
} from '../../../shared/lib/constants'
import { FlightCSSManifest, traverseModules } from './flight-manifest-plugin'
import { FlightCSSManifest } from './flight-manifest-plugin'
import { ASYNC_CLIENT_MODULES } from './flight-manifest-plugin'
import { isClientComponentModule, regexCSS } from '../loaders/utils'
import { traverseModules } from '../utils'

interface Options {
dev: boolean
Expand Down Expand Up @@ -106,27 +107,8 @@ export class FlightClientEntryPlugin {
}
}

compilation.chunkGroups.forEach((chunkGroup) => {
chunkGroup.chunks.forEach((chunk: webpack.Chunk) => {
const chunkModules = compilation.chunkGraph.getChunkModulesIterable(
chunk
) as Iterable<webpack.NormalModule>

for (const mod of chunkModules) {
const modId = compilation.chunkGraph.getModuleId(mod)

recordModule(modId, mod)

// If this is a concatenation, register each child to the parent ID.
// TODO: remove any
const anyModule = mod as any
if (anyModule.modules) {
anyModule.modules.forEach((concatenatedMod: any) => {
recordModule(modId, concatenatedMod)
})
}
}
})
traverseModules(compilation, (mod, _chunk, _chunkGroup, modId) => {
recordModule(modId, mod)
})
})
}
Expand Down
28 changes: 2 additions & 26 deletions packages/next/build/webpack/plugins/flight-manifest-plugin.ts
Expand Up @@ -15,6 +15,8 @@ import {
serverModuleIds,
} from './flight-client-entry-plugin'

import { traverseModules } from '../utils'

// This is the module that will be used to anchor all client references to.
// I.e. it will have all the client files as async deps from this point on.
// We use the Flight client implementation because you can't get to these
Expand Down Expand Up @@ -82,32 +84,6 @@ const PLUGIN_NAME = 'FlightManifestPlugin'
// So that react could unwrap the async module from promise and render module itself.
export const ASYNC_CLIENT_MODULES = new Set<string>()

export function traverseModules(
compilation: webpack.Compilation,
callback: (
mod: any,
chunk: webpack.Chunk,
chunkGroup: typeof compilation.chunkGroups[0]
) => any
) {
compilation.chunkGroups.forEach((chunkGroup) => {
chunkGroup.chunks.forEach((chunk: webpack.Chunk) => {
const chunkModules = compilation.chunkGraph.getChunkModulesIterable(
chunk
// TODO: Update type so that it doesn't have to be cast.
) as Iterable<webpack.NormalModule>
for (const mod of chunkModules) {
callback(mod, chunk, chunkGroup)
const anyModule = mod as any
if (anyModule.modules) {
for (const subMod of anyModule.modules)
callback(subMod, chunk, chunkGroup)
}
}
})
})
}

export class FlightManifestPlugin {
dev: Options['dev'] = false

Expand Down

0 comments on commit fda355d

Please sign in to comment.