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

import-local may not work correctly in a Yarn workspace #7

Open
elliotdavies opened this issue May 14, 2020 · 2 comments
Open

import-local may not work correctly in a Yarn workspace #7

elliotdavies opened this issue May 14, 2020 · 2 comments

Comments

@elliotdavies
Copy link

When using Yarn workspaces, such as in a monorepo setup, node_modules may be split across several locations like so:

- node_modules // workspace root
  - dep@v1
- app1
  - node_modules // nested projects
    - dep@v2
- lib1
  - node_modules

In this scenario an installed dependency may exist at the root node_modules or in one of the nested ones, depending on whether Yarn chooses to hoist it. The dependency may even exist in both places at different version numbers, as illustrated with dep above.

On this line, import-local effectively assumes that the directory it's running from is the sole local directory:

const localFile = resolveCwd.silent(path.join(pkg.name, relativePath));

In a workspace however, import-local could be running from any of the node_modules folders. If for example import-local is installed at the workspace root and gets invoked by dep@v2, import-local will end up finding and invoking dep@v1, because that's in the same directory.

I would say this is a bug because neither version of dep was globally installed, and so I would expect import-local to return false-y.

@misterdjules
Copy link

Thanks for filing this @elliotdavies!

I just ran into the exact same issue when trying to run jest's executable at a given version from a nested directory in a monorepo. Instead, as you described above, it seems that import-local (which jest uses) runs the jest executable from the monorepo's root's node_modules directory, which in my use case can be at a different version.

@goodoldneon
Copy link

I'm running into this. What would the consequences be if we made this change to index.js?

'use strict';
const path = require('path');
const resolveCwd = require('resolve-cwd');
const pkgDir = require('pkg-dir');

module.exports = filename => {
	const globalDir = pkgDir.sync(path.dirname(filename));
	const relativePath = path.relative(globalDir, filename);
	const pkg = require(path.join(globalDir, 'package.json'));
	const localFile = resolveCwd.silent(path.join(pkg.name, relativePath));
	const filenameInLocalNodeModules = isFilenameInLocalNodeModules(filename, localFile);

	// Use `path.relative()` to detect local package installation,
	// because __filename's case is inconsistent on Windows
	// Can use `===` when targeting Node.js 8
	// See https://github.com/nodejs/node/issues/6624
	return !filenameInLocalNodeModules && localFile && path.relative(localFile, filename) !== '' && require(localFile);
};

function isFilenameInLocalNodeModules(filename, localFile) {
	if (localFile !== filename) {
		return true
	}

	const localNodeModules = path.join(process.cwd(), 'node_modules');

	return !path.relative(localNodeModules, filename).startsWith('..')
}

That changes fixes my issue, but I don't know if it creates other problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants