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

refactor: simplify crawlEndFinder #12868

Merged
merged 12 commits into from
Apr 17, 2023
89 changes: 30 additions & 59 deletions packages/vite/src/node/optimizer/optimizer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -711,7 +711,7 @@ async function createDepsOptimizer(
}
}

const runOptimizerIfIdleAfterMs = 50
const callCrawlEndIfIdleAfterMs = 50

interface CrawlEndFinder {
ensureFirstRun: () => void
Expand All @@ -721,18 +721,17 @@ interface CrawlEndFinder {
}

function setupOnCrawlEnd(onCrawlEnd: () => void): CrawlEndFinder {
let registeredIds: { id: string; done: () => Promise<any> }[] = []
const registeredIds = new Set<string>()
const seenIds = new Set<string>()
const workersSources = new Set<string>()
const waitingOn = new Map<string, () => void>()
let firstRunEnsured = false
let crawlEndCalled = false
let timeoutHandle: NodeJS.Timeout | undefined

let cancelled = false
function cancel() {
cancelled = true
}

let crawlEndCalled = false
function callOnCrawlEnd() {
if (!cancelled && !crawlEndCalled) {
crawlEndCalled = true
Expand All @@ -743,84 +742,56 @@ function setupOnCrawlEnd(onCrawlEnd: () => void): CrawlEndFinder {
// If all the inputs are dependencies, we aren't going to get any
// delayDepsOptimizerUntil(id) calls. We need to guard against this
// by forcing a rerun if no deps have been registered
let firstRunEnsured = false
function ensureFirstRun() {
if (!firstRunEnsured && seenIds.size === 0) {
setTimeout(() => {
if (seenIds.size === 0) {
callOnCrawlEnd()
}
}, runOptimizerIfIdleAfterMs)
}, 200)
}
firstRunEnsured = true
}

function registerWorkersSource(id: string): void {
workersSources.add(id)

// Avoid waiting for this id, as it may be blocked by the rollup
// bundling process of the worker that also depends on the optimizer
registeredIds = registeredIds.filter((registered) => registered.id !== id)
registeredIds.delete(id)

const resolve = waitingOn.get(id)
// Forced resolve to avoid waiting for the bundling of the worker to finish
resolve?.()
checkIfCrawlEndAfterTimeout()
}

function delayDepsOptimizerUntil(id: string, done: () => Promise<any>): void {
if (!seenIds.has(id)) {
seenIds.add(id)
registeredIds.push({ id, done })
callOnCrawlEndWhenIdle()
if (!workersSources.has(id)) {
registeredIds.add(id)
done()
.catch(() => {})
Copy link
Member Author

Choose a reason for hiding this comment

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

Because of this catch, which has been around for a long time in different forms, we may be swallowing some load errors during dev. If you remove it, you'll see Vitest complaining about unhandled errors. We need to review this in another PR (this PR doesn't change things but it makes more clear that we are doing it).

.finally(() => markIdAsDone(id))
}
}
}
function markIdAsDone(id: string): void {
registeredIds.delete(id)
checkIfCrawlEndAfterTimeout()
}

async function callOnCrawlEndWhenIdle() {
if (cancelled || waitingOn.size > 0) return

const processingRegisteredIds = registeredIds
registeredIds = []

const donePromises = processingRegisteredIds.map(async (registeredId) => {
// During build, we need to cancel workers
let resolve: () => void
const waitUntilDone = new Promise<void>((_resolve) => {
resolve = _resolve
registeredId
.done()
.catch(() => {
// Ignore errors
})
.finally(() => resolve())
})
waitingOn.set(registeredId.id, () => resolve())

await waitUntilDone
waitingOn.delete(registeredId.id)
})

const afterLoad = () => {
if (cancelled) return
if (
registeredIds.length > 0 &&
registeredIds.every((registeredId) =>
workersSources.has(registeredId.id),
)
) {
return
}

if (registeredIds.length > 0) {
callOnCrawlEndWhenIdle()
} else {
callOnCrawlEnd()
}
}
function checkIfCrawlEndAfterTimeout() {
if (cancelled || registeredIds.size > 0) return

await Promise.allSettled(donePromises)
if (registeredIds.length > 0) {
afterLoad()
} else {
setTimeout(afterLoad, runOptimizerIfIdleAfterMs)
}
if (timeoutHandle) clearTimeout(timeoutHandle)
timeoutHandle = setTimeout(
callOnCrawlEndWhenIdle,
callCrawlEndIfIdleAfterMs,
)
}
async function callOnCrawlEndWhenIdle() {
if (cancelled || registeredIds.size > 0) return
callOnCrawlEnd()
}

return {
Expand Down
1 change: 1 addition & 0 deletions packages/vite/src/node/plugins/optimizedDeps.ts
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ export function optimizedDepsBuildPlugin(config: ResolvedConfig): Plugin {
depsOptimizer.delayDepsOptimizerUntil(resolved.id, async () => {
await this.load(resolved)
})
return resolved
}
}
},
Expand Down
8 changes: 8 additions & 0 deletions playground/optimize-deps/__tests__/optimize-deps.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {
isBuild,
isServe,
page,
serverLogs,
viteTestUrl,
} from '~utils'

Expand Down Expand Up @@ -214,3 +215,10 @@ test('pre bundle css require', async () => {

expect(await getColor('.css-require')).toBe('red')
})

test.runIf(isBuild)('no missing deps during build', async () => {
serverLogs.forEach((log) => {
// no warning from esbuild css minifier
expect(log).not.toMatch('Missing dependency found after crawling ended')
})
})