Feature/#577 support yarn workspaces #598
Conversation
|
❌ Build bit 1.0.2893 failed (commit 455804ec78 by @amitgilad3) |
src/consumer/consumer.js
Outdated
| @@ -603,6 +630,10 @@ export default class Consumer { | |||
| } | |||
| }); | |||
| } | |||
|
|
|||
| // add workspaces if flag is true | |||
| if (this.bitJson.manageWorkspaces) await this.addWorkspacesToPackageJson(); | |||
There was a problem hiding this comment.
you should check that useWorkspaces is true and that packageManager is yarn. otherwise there is no reason to add workspaces
There was a problem hiding this comment.
we should pass the path in case the import is done using -p with a custom path, in this case, we will need to specifically add this path to the workspaces because it won't be under the default directories.
src/consumer/consumer.js
Outdated
| scope: YARN_WORKSPACES_REGEX, | ||
| namespace: YARN_WORKSPACES_REGEX | ||
| }); | ||
| const formatedRegexPath = formatedComponentsPath |
There was a problem hiding this comment.
add comment with example why we did that
src/consumer/consumer.js
Outdated
| .join(DEFAULT_SEPARATOR); | ||
| workSpaces.push(formatedRegexPath); | ||
| pkg.workspaces = R.uniq(workSpaces); | ||
| await pkg.write({ override: true }); |
There was a problem hiding this comment.
you override the existing package.json, what will happen to fields that the user defined but are not supported by our packageJson class in bit-javascript? we should make sure we won't delete anything for the user..
There was a problem hiding this comment.
Amit, regarding Gilad's comment, I had this exact concern when I implemented another task, so in order to keep the root package.json of the user with all the attributes, I didn't use the load() and write() methods of PackageJson.
Instead, I wrote a static method addComponentsIntoExistingPackageJson in the same PackageJson class in bit-javascript, which loads the package.json content raw, add an attribute and save it without creating an instance of PackageJson class.
Feel free to re-use it or write a new similar one.
e2e/e2e-helper.js
Outdated
| const packageJson = { name, version }; | ||
| fs.writeJSONSync(packageJsonPath, packageJson); | ||
| } | ||
| changePackageManagerToYarn( |
There was a problem hiding this comment.
this is confusing, it's change to yarn and also set the manage workspaces.
should be a seperate function.
Also we should add one for the useWorkspaces key.
e2e/e2e-helper.js
Outdated
| fs.writeJSONSync(packageJsonPath, packageJson); | ||
| } | ||
| changePackageManagerToYarn( | ||
| withWorkspaces: boolean = true, |
There was a problem hiding this comment.
change the name to manageWorkspaces
| @@ -1810,7 +1810,64 @@ describe('bit import', function () { | |||
| }); | |||
| }); | |||
| }); | |||
|
|
|||
| describe('import component with dependencies with yarn workspaces', () => { | |||
There was a problem hiding this comment.
add the following cases:
- running it again and make sure the is no duplication in the workspaces
- add some custom field to the package.json and make sure we didn't delete it on the rewrite
- import using -p and make sure the directory has been added to the workspaces
- if manage workspaces is true but packageManager is npm / use workspaces is false, nothing happen
- if the user put some workspaces himself, they shouldn't be overriden
|
❌ Build bit 1.0.2898 failed (commit 437aa940e3 by @amitgilad3) |
|
❌ Build bit 1.0.2903 failed (commit b474f91fd9 by @amitgilad3) |
# Conflicts: # src/consumer/consumer.js
|
❌ Build bit 1.0.2921 failed (commit 01bdcf9cee by @amitgilad3) |
|
❌ Build bit 1.0.2923 failed (commit 763b88dc17 by @amitgilad3) |
…ile and not consumer
|
❌ Build bit 1.0.2934 failed (commit 18de389d84 by @amitgilad3) |
|
❌ Build bit 1.0.2938 failed (commit 20b8d78eee by @amitgilad3) |
|
closes #577 |
#577