Skip to content

feat(cli): warning on relative paths provided by --cwd param#7372

Open
mollyIV wants to merge 2 commits intoyarnpkg:masterfrom
mollyIV:add-command-relative-path-warning
Open

feat(cli): warning on relative paths provided by --cwd param#7372
mollyIV wants to merge 2 commits intoyarnpkg:masterfrom
mollyIV:add-command-relative-path-warning

Conversation

@mollyIV
Copy link
Copy Markdown

@mollyIV mollyIV commented Jul 4, 2019

Summary

When a user executes yarn add --cwd relative-path-to-the-dir dependency-name command and passes a relative path to the directory, the path is resolved and a current working directory is used instead.

this.cwd = path.resolve(opts.cwd || this.cwd || process.cwd());

A user might be surprised by described behaviour, so adding a warning can be really helpful. You can find more details about it in this issue.

Test plan

I did perform manual testing for a relative path:

Screen Shot 2019-07-04 at 10 14 20

The new warning is added ☝️🎊

Also tested the potential regression, if we passed an absolute path to the command:

Screen Shot 2019-07-04 at 10 16 09

Worked as expected 👍

I wish I could add unit tests to check the warning. Any suggestion on that would be appreciated 🙏 Was thinking about adding it to config.js tests - checking the warning while config initialisation for a relative path.

closes #5206

Comment thread src/reporters/lang/en.js
yarnOutdatedCommand: 'To upgrade, run the following command:',

yarnAddRelativePathDetected:
'Detected a relative path. --cwd only supports an absolute path. A current working directory will be used instead.',
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Opened to better message suggestions 🙌

@buildsize
Copy link
Copy Markdown

buildsize Bot commented Jul 4, 2019

File name Previous Size New Size Change
yarn-[version].noarch.rpm 1.18 MB 1.18 MB 7 bytes (0%)
yarn-[version].js 4.85 MB 4.85 MB 319 bytes (0%)
yarn-legacy-[version].js 5.04 MB 5.05 MB 319 bytes (0%)
yarn-v[version].tar.gz 1.18 MB 1.19 MB 2.01 KB (0%)
yarn_[version]all.deb 868.29 KB 868.67 KB 392 bytes (0%)

Comment thread src/config.js

if (opts.cwd !== undefined) {
if (!path.isAbsolute(String(opts.cwd))) {
this.reporter.warn(this.reporter.lang('yarnAddRelativePathDetected'));
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe the add command should display this warning only in a verbose mode? 🤔

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

Successfully merging this pull request may close these issues.

yarn add --cwd invalid-dir should warn/fail

1 participant