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

fix(modules-folder): Fix --modules-folder handling in several places. #6850

Merged
merged 6 commits into from Feb 28, 2019
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 5 additions & 1 deletion CHANGELOG.md
Expand Up @@ -4,11 +4,15 @@ Please add one entry in this file for each change in Yarn's behavior. Use the sa

## Master

- Fixes `--modules-folder` handling in several places (ex: `yarn check` now respects `--modules-folder`)

[#6850](https://github.com/yarnpkg/yarn/pull/6850) - [**Jeff Valore**](https://twitter.com/codingwithspike)

- Removes `rootModuleFolders` (internal variable which wasn't used anywhere)

[#6846](https://github.com/yarnpkg/yarn/pull/6846) - [**Jeff Valore**](https://twitter.com/codingwithspike)

- Allows `global-folder` to be set in `.yarnrc` files
- Adds support for setting `global-folder` from `.yarnrc` files

[#7056](https://github.com/yarnpkg/yarn/pull/7056) - [**Hsiao-nan Cheung**](https://github.com/niheaven)

Expand Down
1 change: 1 addition & 0 deletions __tests__/commands/_helpers.js
Expand Up @@ -97,6 +97,7 @@ export function makeConfigFromDirectory(cwd: string, reporter: Reporter, flags:
focus: !!flags.focus,
enableDefaultRc: !flags.noDefaultRc,
extraneousYarnrcFiles: flags.useYarnrc,
modulesFolder: flags.modulesFolder ? path.join(cwd, flags.modulesFolder) : undefined,
},
reporter,
);
Expand Down
13 changes: 13 additions & 0 deletions __tests__/commands/install/integration.js
Expand Up @@ -1140,6 +1140,19 @@ test('install will not warn for missing optional peer dependencies', () =>
expect(warningMessage).toBe(false);
}));

test('does not check node_modules for extraneous files when --modules-folder used', async () => {
// Scenario: https://github.com/yarnpkg/yarn/issues/5419
// When `--modules-foler` is passed, yarn should check that directory for extraneous files.
// Also, the default node_modules dir, if it exists, should not be cleaned out (marked as extraneous).
await runInstall({modulesFolder: './some_modules'}, 'extraneous-node-modules', async (config): Promise<void> => {
expect(await fs.exists(`${config.cwd}/some_modules/feed`)).toEqual(true);
// Extraneous files in node_modules should not have been cleaned.
expect(await fs.exists(`${config.cwd}/node_modules/extra.js`)).toEqual(true);
// Extraneous files in some_modules should have been cleaned.
expect(await fs.exists(`${config.cwd}/some_modules/extra.js`)).toEqual(false);
});
});

test('install skips the scripts if the yarnrc specifies skip-scripts true', () =>
runInstall({}, 'ignore-scripts-by-yarnrc', (config, reporter, install, getStdout) => {
const stdout = getStdout();
Expand Down
Empty file.
@@ -0,0 +1,5 @@
{
"dependencies": {
"feed": "0.3.0"
}
}
Empty file.
8 changes: 2 additions & 6 deletions src/cli/commands/autoclean.js
Expand Up @@ -78,12 +78,8 @@ export async function clean(

// build list of possible module folders
const locs = new Set();
if (config.modulesFolder) {
locs.add(config.modulesFolder);
}
for (const name of registryNames) {
const registry = config.registries[name];
locs.add(path.join(config.lockfileFolder, registry.folder));
for (const registryFolder of config.registryFolders) {
locs.add(path.resolve(config.lockfileFolder, registryFolder));
}

const workspaceRootFolder = config.workspaceRootFolder;
Expand Down
3 changes: 1 addition & 2 deletions src/cli/commands/bin.js
Expand Up @@ -2,7 +2,6 @@

import type {Reporter} from '../../reporters/index.js';
import type Config from '../../config.js';
import RegistryYarn from '../../resolvers/registries/yarn-resolver.js';
import {getBinEntries} from './run.js';

const path = require('path');
Expand All @@ -16,7 +15,7 @@ export function setFlags(commander: Object) {
}

export async function run(config: Config, reporter: Reporter, flags: Object, args: Array<string>): Promise<void> {
const binFolder = path.join(config.cwd, config.registries[RegistryYarn.registry].folder, '.bin');
const binFolder = path.join(config.cwd, config.registryFolders[0], '.bin');
if (args.length === 0) {
reporter.log(binFolder, {force: true});
} else {
Expand Down
12 changes: 6 additions & 6 deletions src/cli/commands/check.js
Expand Up @@ -39,7 +39,7 @@ export async function verifyTreeCheck(
}
// check all dependencies recursively without relying on internal resolver
const registryName = 'yarn';
const registry = config.registries[registryName];
const registryFolder = config.registryFolders[0];
const cwd = config.workspaceRootFolder ? config.lockfileFolder : config.cwd;
const rootManifest = await config.readManifest(cwd, registryName);

Expand Down Expand Up @@ -86,7 +86,7 @@ export async function verifyTreeCheck(
const locationsVisited: Set<string> = new Set();
while (dependenciesToCheckVersion.length) {
const dep = dependenciesToCheckVersion.shift();
const manifestLoc = path.join(dep.parentCwd, registry.folder, dep.name);
const manifestLoc = path.resolve(dep.parentCwd, registryFolder, dep.name);
if (locationsVisited.has(manifestLoc + `@${dep.version}`)) {
continue;
}
Expand Down Expand Up @@ -114,19 +114,19 @@ export async function verifyTreeCheck(
const dependencies = pkg.dependencies;
if (dependencies) {
for (const subdep in dependencies) {
const subDepPath = path.join(manifestLoc, registry.folder, subdep);
const subDepPath = path.resolve(manifestLoc, registryFolder, subdep);
let found = false;
const relative = path.relative(cwd, subDepPath);
const locations = path.normalize(relative).split(registry.folder + path.sep).filter(dir => !!dir);
const locations = path.normalize(relative).split(registryFolder + path.sep).filter(dir => !!dir);
locations.pop();
while (locations.length >= 0) {
let possiblePath;
if (locations.length > 0) {
possiblePath = path.join(cwd, registry.folder, locations.join(path.sep + registry.folder + path.sep));
possiblePath = path.join(cwd, registryFolder, locations.join(path.sep + registryFolder + path.sep));
} else {
possiblePath = cwd;
}
if (await fs.exists(path.join(possiblePath, registry.folder, subdep))) {
if (await fs.exists(path.resolve(possiblePath, registryFolder, subdep))) {
dependenciesToCheckVersion.push({
name: subdep,
originalKey: `${dep.originalKey}#${subdep}`,
Expand Down
5 changes: 2 additions & 3 deletions src/cli/commands/run.js
Expand Up @@ -5,7 +5,6 @@ import type Config from '../../config.js';
import {execCommand, makeEnv} from '../../util/execute-lifecycle-script.js';
import {dynamicRequire} from '../../util/dynamic-require.js';
import {MessageError} from '../../errors.js';
import {registries} from '../../resolvers/index.js';
import * as fs from '../../util/fs.js';
import * as constants from '../../constants.js';

Expand All @@ -29,8 +28,8 @@ export async function getBinEntries(config: Config): Promise<Map<string, string>
const binEntries = new Map();

// Setup the node_modules/.bin folders for analysis
for (const registry of Object.keys(registries)) {
binFolders.add(path.join(config.cwd, config.registries[registry].folder, '.bin'));
for (const registryFolder of config.registryFolders) {
binFolders.add(path.resolve(config.lockfileFolder, registryFolder, '.bin'));
}

// Same thing, but for the pnp dependencies, located inside the cache
Expand Down
2 changes: 1 addition & 1 deletion src/config.js
Expand Up @@ -317,7 +317,7 @@ export default class Config {
}

if (this.modulesFolder) {
this.registryFolders.push(this.modulesFolder);
this.registryFolders = [this.modulesFolder];
}

this.networkConcurrency =
Expand Down
8 changes: 2 additions & 6 deletions src/util/execute-lifecycle-script.js
Expand Up @@ -7,7 +7,6 @@ import * as child from './child.js';
import * as fs from './fs.js';
import {dynamicRequire} from './dynamic-require.js';
import {makePortableProxyScript} from './portable-script.js';
import {registries} from '../resolvers/index.js';
import {fixCmdWinSlashes} from './fix-cmd-win-slashes.js';
import {getBinFolder as getGlobalBinFolder, run as globalRun} from '../cli/commands/global.js';

Expand Down Expand Up @@ -191,16 +190,13 @@ export async function makeEnv(
}

// Add node_modules .bin folders to the PATH
for (const registry of Object.keys(registries)) {
const binFolder = path.join(config.registries[registry].folder, '.bin');
for (const registryFolder of config.registryFolders) {
const binFolder = path.join(registryFolder, '.bin');
if (config.workspacesEnabled && config.workspaceRootFolder) {
pathParts.unshift(path.join(config.workspaceRootFolder, binFolder));
}
pathParts.unshift(path.join(config.linkFolder, binFolder));
pathParts.unshift(path.join(cwd, binFolder));
if (config.modulesFolder) {
pathParts.unshift(path.join(config.modulesFolder, '.bin'));
}
}

let pnpFile;
Expand Down