Skip to content

Commit

Permalink
Resolves folder options (i.e. --modules-folder) relative to cwd (yarn…
Browse files Browse the repository at this point in the history
…pkg#7074)

* Prevents potentially surprising behavior and data loss

* Includes integration test verifying that user-reported data loss scenario does not occur
  • Loading branch information
Matthew Preble committed Oct 7, 2019
1 parent c040e79 commit 9c153f8
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 6 deletions.
33 changes: 33 additions & 0 deletions __tests__/integration.js
Original file line number Diff line number Diff line change
Expand Up @@ -560,3 +560,36 @@ test('yarn init -y', async () => {
const manifestFile = await fs.readFile(path.join(cwd, 'package.json'));
expect(manifestFile).toEqual(initialManifestFile);
});

test('--modules-folder option', async () => {
/**
* The behavior of --modules-folder (and other folder options) was that it resolved relative not to the current
* working directory, but instead to the closest project root (folder containing a package.json file).
*
* This behavior was at best surprising and could result in data loss. This test captures a scenario in which
* there would previously have been data loss, demonstrating the fix for --modules-folder and other folder options.
*
*/
const projectFolder = await makeTemp();
const libraryFolder = path.join(projectFolder, 'lib');

const initialManifestFile = JSON.stringify({name: 'test', license: 'ISC', version: '1.0.0'});
const importantData = 'I definitely care about this file!';

await fs.writeFile(`${projectFolder}/package.json`, initialManifestFile);
await fs.writeFile(`${projectFolder}/IMPORTANT_FILE.txt`, importantData);
await fs.mkdirp(libraryFolder);

const options = {cwd: libraryFolder};

// This yarn command fails with the previous behavior, the rest of the test is defense in depth
await runYarn(['add', 'left-pad', '--modules-folder', '.'], options);

// Dependencies should have been installed in the 'lib' folder
const libraryFolderContents = await fs.readdir(`${libraryFolder}`);
expect(libraryFolderContents).toContain('left-pad');

// Additionally, there should have not been any data loss in the project folder
const importantFile = await fs.readFile(`${projectFolder}/IMPORTANT_FILE.txt`);
expect(importantFile).toBe(importantData);
});
17 changes: 11 additions & 6 deletions src/cli/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -509,21 +509,26 @@ export async function main({

const cwd = command.shouldRunInCurrentCwd ? commander.cwd : findProjectRoot(commander.cwd);

const folderOptionKeys = ['linkFolder', 'globalFolder', 'preferredCacheFolder', 'cacheFolder', 'modulesFolder'];

// Resolve all folder options relative to cwd
const resolvedFolderOptions = {};
folderOptionKeys.forEach(folderOptionKey => {
const folderOption = commander[folderOptionKey];
const resolvedFolderOption = folderOption ? path.resolve(commander.cwd, folderOption) : folderOption;
resolvedFolderOptions[folderOptionKey] = resolvedFolderOption;
});

await config
.init({
cwd,
commandName,

...resolvedFolderOptions,
enablePnp: commander.pnp,
disablePnp: commander.disablePnp,
enableDefaultRc: commander.defaultRc,
extraneousYarnrcFiles: commander.useYarnrc,
binLinks: commander.binLinks,
modulesFolder: commander.modulesFolder,
linkFolder: commander.linkFolder,
globalFolder: commander.globalFolder,
preferredCacheFolder: commander.preferredCacheFolder,
cacheFolder: commander.cacheFolder,
preferOffline: commander.preferOffline,
captureHar: commander.har,
ignorePlatform: commander.ignorePlatform,
Expand Down

0 comments on commit 9c153f8

Please sign in to comment.