Adds bolt add and bolt workspace add commands
#35
Conversation
|
[] refactor nested-workspaces-with-root-dependencies-installed tests to use |
| @@ -6,6 +6,6 @@ | |||
| }, | |||
| "dependencies": { | |||
| "react": "^15.6.1", | |||
| "bar": "^1.0.0" | |||
| "@scoped/bar": "^1.0.0" | |||
There was a problem hiding this comment.
Hmm... I'm going to double check all these fixures. I dont recall changing all these, so I'm thinking my tests may have modified them at some point.
- find out if I modified all these fixtures or not...
src/commands/__tests__/add.test.js
Outdated
| } | ||
|
|
||
| // a mock yarn add function to update the pakcages's config and also node_modules dir | ||
| async function fakeYarnAdd(pkg: Package, dependencies, type = 'dependencies') { |
There was a problem hiding this comment.
This is the yarn.add mock I'll be using in the rest of the tests later. Just a much cleaner way of handling than what I was doing before.
src/commands/add.js
Outdated
| type: configDependencyType | ||
| }; | ||
|
|
||
| const depTypeFlags = { |
There was a problem hiding this comment.
This and the toWorkspaceAddOptions are almost identical to the ones in add() bar the workspace arg. I'd love to dedupe if possible...
|
Okay, had another look through and this seems pretty fine. I'm going to start work on the addDependenciesToPackage s function now. |
src/utils/yarn.js
Outdated
| devDependencies: '--dev', | ||
| peerDependencies: '--peer', | ||
| optionalDependencies: '--optional' | ||
| }; |
There was a problem hiding this comment.
Might as well hoist this as a constant
| ); | ||
|
|
||
| await yarn.run(pkg, 'postinstall'); | ||
| await yarn.run(pkg, 'prepublish'); |
There was a problem hiding this comment.
| pkg.config.getName(), | ||
| dep.name, | ||
| installed, | ||
| String(dep.version) |
There was a problem hiding this comment.
If you store the version in a variable, you won't need to check this again.
const installed = project.pkg.getDependencyVersionRange(dep.name);
const version = dep.version;
if (version && version !== installed) {
logger.warn(
messages.depMustMatchProject(
pkg.config.getName(),
dep.name,
installed,
version
)
);
| String(dep.version) | ||
| ) | ||
| ); | ||
| throw new Error(); |
There was a problem hiding this comment.
I'd rather throw the warning as an error instead of logging it and throwing an error.
| String(dep.version) | ||
| ) | ||
| ); | ||
| throw new Error(); |
There was a problem hiding this comment.
Same as above https://github.com/boltpkg/bolt/pull/35/files#r143371033
|
I think we should enforce at the project level that you cannot add a workspace as a dependency. path/to/project/ $ bolt add workspace-name # error
path/to/project/other-workspace/ $ bolt add workspace-name # success |
| }) | ||
| ); | ||
| expect(yarn.add).toHaveBeenCalledTimes(0); | ||
| expect(await depIsInstalled(fooWorkspaceDir, 'project-only-dep')).toEqual( |
There was a problem hiding this comment.
btw .toEqual is meant for deep object comparison where .toBe is meant for reference equality. I generally use it for .toBe(true)-etc
There was a problem hiding this comment.
Oh, no worries. I'll go replace all of these in the next PR if you like
|
Can you rebase? |
|
I cant seem to push here after rebasing, so pushed to #36 |
This is encompasses all the work from #27
Its still a bit messy and theres some dupliacted code I'd like to clean up, but I wanna try to get this in and do any more refactors in the next drop (which would also add
bolt workspaces add).As an aside, the testing patterns I've been using have changed a fair bit over the course of all this, so I definitely want to refactor and align all that at some point.
The mock yarn.add command for instance is in three different forms in three different places. A single one that everything uses would be awesome.