Apparent race condition in recursive resolveId logic

[sophiebits]

• Read the docs.
• Use Vite >=2.0.
• Not upgrading from 1.x.

---

First up, sorry in advance – I don't have a good repro case for this.

While exploring adopting Vite in a large codebase I ran into a mysterious error with an import like this in an SCSS file:

// in _abc.scss
@import './_xyz';

This is supposed to resolve to _xyz.scss with the extension, but I was receiving an error from this line of code

vite/packages/vite/src/node/plugins/css.ts

Line 869 in 5b56d70

const content = fs.readFileSync(file, 'utf-8')

where it said† it couldn't find the file /path/to/_xyz (without the expected .scss extension). Since the extension was missing, of course it couldn't find it.

(node:19771) UnhandledPromiseRejectionWarning: Error: ENOENT: no such file or directory, open '/path/to/_xyz'
    at Object.openSync (fs.js:476:3)
    at Object.readFileSync (fs.js:377:35)
    at rebaseUrls (/path/to/node_modules/vite/dist/node/chunks/dep-be3111d5.js:26218:33)
    at /path/to/node_modules/vite/dist/node/chunks/dep-be3111d5.js:26168:21

A little logging confirmed that this call to resolvers.sass(...)

vite/packages/vite/src/node/plugins/css.ts

Line 808 in 5b56d70

resolvers.sass(url, importer).then((resolved) => {

was sometimes returning /path/to/_xyz and sometimes returning /path/to/_xyz.scss – despite it being the same exact function called with the same arguments (and configured properly at its instantiation)!

The Race Condition

I dug in a bit further to find this code, which smelled a bit fishy.

vite/packages/vite/src/node/server/pluginContainer.ts

Lines 419 to 439 in 5b56d70

	if (_skip) {
	if (_skip.includes(plugin)) continue
	if (resolveSkips.has(plugin, key)) continue
	resolveSkips.add(plugin, key)
	}
	ctx._activePlugin = plugin
	let result
	const pluginResolveStart = isDebug ? Date.now() : 0
	try {
	result = await plugin.resolveId.call(
	ctx as any,
	rawId,
	importer,
	{},
	ssr
	)
	} finally {
	if (_skip) resolveSkips.delete(plugin, key)
	}

Note how we mutate the global resolveSkips map before and after calling plugin.resolveId – but since that call is async, arbitrary other code can run at the same time! On a hunch, I tried changing this code to use Node's AsyncLocalStorage to allocate a new resolveSkips map in the outermost call to resolveId and pass it down to the child calls, so that simultaneous calls to resolveId don't interfere. (Note that nestedResolveCall variable is similarly wrong, but it's not used for any important logic so it doesn't really cause a problem.)

Making the change to use AsyncLocalStorage 100% fixed the problem for me. This issue doesn't come with a PR from me because (a) AsyncLocalStorage isn't available natively until Node 12.7.0 and Vite supports 12.0.0 now, (b) not sure if there deserves to be a larger refactor here to thread through the context the old-fashioned way – I trust y'all are better judges of that. But probably one of those solutions should be taken here.

System Info

• vite version: 2.0.0-beta.65
• Operating System: macOS High Sierra 10.13.6
• Node version: v14.15.4
• Package manager (npm/yarn/pnpm) and version: yarn 1.16.0

---

† It so happens that this is an UnhandledPromiseRejection and gives a bad error message (eg: no clue as to what file was being processed at the time, does not stop the build, logs a bazillion times) – which happens because exceptions in rebaseUrls here are not properly returned:

vite/packages/vite/src/node/plugins/css.ts

Line 810 in 5b56d70

rebaseUrls(resolved, options.filename).then(done)

If I reject the Promise that gets returned, seems to be a better error. Not what this issue is about though.

[yyx990803]

Thanks for the detailed report! I think this probably affects WMR as well, /cc @developit

[developit]

Yep - time to ditch the singleton, haha.

I'm a little wary if ALS for this since I haven't used it yet. My gut says this might be best solved by creating a plugin execution context per module processed. Thoughts?

[yyx990803]

Fix in 85f1e7b is probably incorrect. I need to craft a dedicated test case for this.

[sophiebits]

Thank you! I can confirm that on 2.0.0-beta.66 I no longer experience the problem in my project.