[Bug] yarn add dep & devDep duplicate the dependency.

[CallMeLaNN]

Describe the bug

Sometime I forgot to add --dev when trying to add devDependencies. Doing so again will duplicate the dependency entry in package.json.

IMO, v1 will stop us and ask to remove the package first. For v2, we can just automatically move it.

To Reproduce

const {promises: {readFile}} = require(`fs`);

await packageJsonAndInstall({
  devDependencies: {lodash: `*`},
});

await yarn(`add`, `lodash`);

const pkgJson = JSON.parse(await readFile(`package.json`, `utf8`));
expect(pkgJson.dependencies).not.toHaveProperty(`lodash`);

Environment if relevant (please complete the following information):

• OS: OSX
• Node version: 10.16.2
• Yarn version: 2.0.0-rc1 (tag 2019-08-16)

[yarnbot]

This issue reproduces on master:

Error: expect(received).not.toHaveProperty(path)

Expected path: not "lodash"

Received value: "^4.17.15"
    at module.exports (evalmachine.<anonymous>:11:34)

[arcanis]

Yep, the current behavior is definitely incorrect.

What we should do is less obvious however - my personal preference would be to just apply the upgrade to whatever field was previously used (only regular and dev, never peer), except if an explicit flag would cause a conflict (in which case it should fail).

So:

• yarn add lodash when lodash is a regular dep would upgrade it in dependencies
• yarn add lodash when lodash is a dev dep would upgrade it in devDependencies
• yarn add lodash --dev when lodash is a regular dep would cause an error
• yarn add lodash --prefer-dev when lodash is a regular dep would upgrade it in dependencies
• yarn add lodash --prefer-dev when lodash isn't a dependency would add it to devDependencies
• yarn add lodash when lodash is a peer dep would cause an error
• yarn add lodash --dev when lodash is a peer dep would work
• yarn add lodash --peer when lodash is a regular dependency would cause an error
• yarn add lodash --peer when lodash is a dev dependency would work

The changes would have do be done around here:

https://github.com/yarnpkg/berry/blob/master/packages/plugin-essentials/sources/commands/add.ts#L81

[AvrahamO]

I would love to contribute to this project.
Started digging into the code and experimenting with it, and I have some questions:
According to your spec, what should happen if user types yarn add lodash react, when lodash is already a dev dep, but react is new?
Also, in case when user added lodash without --dev by mistake, how would he easily fix this mistake?

[arcanis]

Welcome @AvrahamO!

According to your spec, what should happen if user types yarn add lodash react, when lodash is already a dev dep, but react is new?

I think each one should be taken in isolation. So react would become a regular dependency, and lodash would stay a dev dependency.

Also, in case when user added lodash without --dev by mistake, how would he easily fix this mistake?

They can edit the package.json to fix their metadata and run yarn install again.

[arcanis]

Fixed by @AvrahamO in #428 🎉