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

perf(esbuild): make tsconfck non-blocking #12548

Merged
merged 2 commits into from
Mar 23, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
96 changes: 58 additions & 38 deletions packages/vite/src/node/plugins/esbuild.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import type {
import { transform } from 'esbuild'
import type { RawSourceMap } from '@ampproject/remapping'
import type { InternalModuleFormat, SourceMap } from 'rollup'
import type { TSConfckParseOptions, TSConfckParseResult } from 'tsconfck'
import type { TSConfckParseOptions } from 'tsconfck'
import { TSConfckParseError, findAll, parse } from 'tsconfck'
import {
cleanUrl,
Expand All @@ -18,11 +18,13 @@ import {
createFilter,
ensureWatchedFile,
generateCodeFrame,
timeFrom,
} from '../utils'
import type { ResolvedConfig, ViteDevServer } from '..'
import type { Plugin } from '../plugin'
import { searchForWorkspaceRoot } from '..'

const isDebug = process.env.DEBUG
const debug = createDebugger('vite:esbuild')

const INJECT_HELPERS_IIFE_RE =
Expand Down Expand Up @@ -204,7 +206,8 @@ export async function transformWithEsbuild(
}
}

export function esbuildPlugin(options: ESBuildOptions): Plugin {
export function esbuildPlugin(config: ResolvedConfig): Plugin {
const options = config.esbuild as ESBuildOptions
const { jsxInject, include, exclude, ...esbuildTransformOptions } = options

const filter = createFilter(include || /\.(m?ts|[jt]sx)$/, exclude || /\.js$/)
Expand All @@ -226,6 +229,8 @@ export function esbuildPlugin(options: ESBuildOptions): Plugin {
keepNames: false,
}

initTSConfck(config.root)

return {
name: 'vite:esbuild',
configureServer(_server) {
Expand All @@ -235,9 +240,6 @@ export function esbuildPlugin(options: ESBuildOptions): Plugin {
.on('change', reloadOnTsconfigChange)
.on('unlink', reloadOnTsconfigChange)
},
async configResolved(config) {
await initTSConfck(config)
},
buildEnd() {
// recycle serve to avoid preventing Node self-exit (#6815)
server = null as any
Expand Down Expand Up @@ -281,11 +283,10 @@ const rollupToEsbuildFormatMap: Record<
}

export const buildEsbuildPlugin = (config: ResolvedConfig): Plugin => {
initTSConfck(config.root)

return {
name: 'vite:esbuild-transpile',
async configResolved(config) {
await initTSConfck(config)
},
async renderChunk(code, chunk, opts) {
// @ts-expect-error injected by @vitejs/plugin-legacy
if (opts.__vite_skip_esbuild__) {
Expand Down Expand Up @@ -432,32 +433,51 @@ function prettifyMessage(m: Message, code: string): string {
return res + `\n`
}

const tsconfckParseOptions: TSConfckParseOptions = {
cache: new Map<string, TSConfckParseResult>(),
tsConfigPaths: undefined,
root: undefined,
resolveWithEmptyIfConfigNotFound: true,
let tsconfckRoot: string | undefined
let tsconfckParseOptions: TSConfckParseOptions | Promise<TSConfckParseOptions> =
{ resolveWithEmptyIfConfigNotFound: true }
Comment on lines +436 to +438
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't we have these cache as WeakMap<ResolvedConfig,...>? It would be good to make it safe to create two esbuild plugins programmatically.

Copy link
Member Author

Choose a reason for hiding this comment

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

transformWithEsbuild mess it all up 😄 It doesn't have a reference to ResolvedConfig, and we can't get the "first value" of the weak map because the API doesn't support that.

I refactored this part a few times and decided to keep most of it as is at the end. Maybe we can hae transformWithEsbuild accept a ResolvedConfig too in the future.


function initTSConfck(root: string, force = false) {
// bail if already cached
if (!force && root === tsconfckRoot) return

const workspaceRoot = searchForWorkspaceRoot(root)

tsconfckRoot = root
tsconfckParseOptions = initTSConfckParseOptions(workspaceRoot)

// cached as the options value itself when promise is resolved
tsconfckParseOptions.then((options) => {
if (root === tsconfckRoot) {
tsconfckParseOptions = options
}
})
}

async function initTSConfck(config: ResolvedConfig) {
const workspaceRoot = searchForWorkspaceRoot(config.root)
debug(`init tsconfck (root: ${colors.cyan(workspaceRoot)})`)

tsconfckParseOptions.cache!.clear()
tsconfckParseOptions.root = workspaceRoot
tsconfckParseOptions.tsConfigPaths = new Set([
...(await findAll(workspaceRoot, {
skip: (dir) => dir === 'node_modules' || dir === '.git',
})),
])
debug(`init tsconfck end`)
async function initTSConfckParseOptions(workspaceRoot: string) {
const start = isDebug ? performance.now() : 0

const options: TSConfckParseOptions = {
cache: new Map(),
root: workspaceRoot,
tsConfigPaths: new Set(
await findAll(workspaceRoot, {
skip: (dir) => dir === 'node_modules' || dir === '.git',
}),
),
resolveWithEmptyIfConfigNotFound: true,
}

isDebug && debug(timeFrom(start), 'tsconfck init', colors.dim(workspaceRoot))

return options
}

async function loadTsconfigJsonForFile(
filename: string,
): Promise<TSConfigJSON> {
try {
const result = await parse(filename, tsconfckParseOptions)
const result = await parse(filename, await tsconfckParseOptions)
// tsconfig could be out of root, make sure it is watched on dev
if (server && result.tsconfigFile !== 'no_tsconfig_file_found') {
ensureWatchedFile(server.watcher, result.tsconfigFile, server.config.root)
Expand All @@ -474,15 +494,15 @@ async function loadTsconfigJsonForFile(
}
}

function reloadOnTsconfigChange(changedFile: string) {
async function reloadOnTsconfigChange(changedFile: string) {
// server could be closed externally after a file change is detected
if (!server) return
// any tsconfig.json that's added in the workspace could be closer to a code file than a previously cached one
// any json file in the tsconfig cache could have been used to compile ts
if (
path.basename(changedFile) === 'tsconfig.json' ||
(changedFile.endsWith('.json') &&
tsconfckParseOptions?.cache?.has(changedFile))
(await tsconfckParseOptions)?.cache?.has(changedFile))
) {
server.config.logger.info(
`changed tsconfig file detected: ${changedFile} - Clearing cache and forcing full-reload to ensure TypeScript is compiled with updated config values.`,
Expand All @@ -493,15 +513,15 @@ function reloadOnTsconfigChange(changedFile: string) {
server.moduleGraph.invalidateAll()

// reset tsconfck so that recompile works with up2date configs
initTSConfck(server.config).finally(() => {
// server may not be available if vite config is updated at the same time
if (server) {
// force full reload
server.ws.send({
type: 'full-reload',
path: '*',
})
}
})
initTSConfck(server.config.root, true)

// server may not be available if vite config is updated at the same time
if (server) {
// force full reload
server.ws.send({
type: 'full-reload',
path: '*',
})
}
}
Comment on lines -496 to 526
Copy link
Member Author

Choose a reason for hiding this comment

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

Since tsconfck is non-blocking now, we can reload the page right away, those that rely on tsconfig will wait for the promise before continuing.

}
2 changes: 1 addition & 1 deletion packages/vite/src/node/plugins/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ export async function resolvePlugins(
}),
htmlInlineProxyPlugin(config),
cssPlugin(config),
config.esbuild !== false ? esbuildPlugin(config.esbuild) : null,
config.esbuild !== false ? esbuildPlugin(config) : null,
jsonPlugin(
{
namedExports: true,
Expand Down