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

Support .ava cacheDir for better compatibility with Yarn PnP #3226

Open
andrewzey opened this issue Jul 28, 2023 · 1 comment
Open

Support .ava cacheDir for better compatibility with Yarn PnP #3226

andrewzey opened this issue Jul 28, 2023 · 1 comment

Comments

@andrewzey
Copy link

Hello,

We're using Yarn 3 with PnP in our mono-repo, and find that ava is creating unwanted node_modules directories in our workspace (with node_modules/.cache/ava/failing-tests.json) This is a result of

ava/lib/cli.js

Line 256 in f047694

const cacheDir = path.join(projectDir, 'node_modules', '.cache', 'ava');

Sometimes the existence of node_modules confuses the pnp patched executables (eg. eslint), so the mere existence of node_modules can sometimes cause issues with things working reliable.

It would be great if you supported some manner of specifying the cache directory or simply defaulted to projectDir/.ava and recommended adding .ava to the .gitignore.

Thanks!

@andrewzey andrewzey changed the title Consider allowing specifying cacheDir for better compatibility with Yarn PnP Consider allowing specifying cacheDir or at least no node_modules for better compatibility with Yarn PnP Jul 28, 2023
@novemberborn
Copy link
Member

I was thinking about this the other day actually, and yes I agree that should be an option. Placing it under node_modules by default does provide the best out-of-the box experience though.

cache is currently configurable as a boolean, I propose we add '.ava' as a valid value which would then use path.join(projectDir, '.ava') as the cache dir. All other values should result in a configuration error, I don't think we should make this super configurable just yet.


However this does leave us with the following code:

ava/lib/worker/base.js

Lines 210 to 217 in 4dc385b

let importFromProject = async ref => {
// Do not use the cacheDir since it's not guaranteed to be inside node_modules.
const avaCacheDir = joinPath(projectDir, 'node_modules', '.cache', 'ava');
await mkdir(avaCacheDir, {recursive: true});
const stubPath = joinPath(avaCacheDir, 'import-from-project.mjs');
await writeFileAtomic(stubPath, 'export const importFromProject = ref => import(ref);\n');
({importFromProject} = await import(pathToFileURL(stubPath)));
return importFromProject(ref);

The way this could work is:

  • If cache is not false, use the computed cacheDir
  • Otherwise, insist on using this node_modules/.cache/ava directory

And then we'd have to document that as a caveat.

What do you think?

@novemberborn novemberborn changed the title Consider allowing specifying cacheDir or at least no node_modules for better compatibility with Yarn PnP Support .ava cacheDir for better compatibility with Yarn PnP Jul 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants