feat: install cmd supports monorepo #4140
Conversation
|
Any tests we can add to verify this? |
cli/src/lib/npm/npmProjectUtils.js
Outdated
| ): Promise<string[]> { | ||
| if ( | ||
| pkgJson.content.private !== true || | ||
| !Array.isArray(pkgJson.content.workspaces) |
There was a problem hiding this comment.
It seems that yarn has updated their api over time, but my projects seem to use workspaces.packages can you make it support that also so that flow-typed has the best compatibility with the end user?
There was a problem hiding this comment.
Sorry, I don't really understand your question.
workspaces field seems to be string[] according to:
- npm: https://docs.npmjs.com/cli/v7/using-npm/workspaces
- yarn: https://classic.yarnpkg.com/lang/en/docs/workspaces
Do you mean you have, workspaces.packages field?
https://docs.npmjs.com/cli/v7/configuring-npm/package-json#workspaces
There was a problem hiding this comment.
Yeah I think docs have since updated but they still support the old way. I implemented workspaces way back when it was first released and never really touched it since, my package.json looks like this
"workspaces": {
"packages": [
"packages/*"
]
},
Do you think we should continue to support this so that if I want to make use of this I also don't need to change the way my project is configured?
cli/src/commands/install.js
Outdated
| @@ -265,7 +266,17 @@ async function installNpmLibDefs({ | |||
| const termMatches = term.match(/(@[^@\/]+\/)?([^@]+)@(.+)/); | |||
| if (termMatches == null) { | |||
| const pkgJsonData = await getPackageJsonData(cwd); | |||
| const pkgJsonDeps = getPackageJsonDependencies(pkgJsonData, []); | |||
| const workspacesPckJsonData = await findWorkspacesPackages( | |||
There was a problem hiding this comment.
Nitpick but suggest we rename this var to be consistent with how it's used in rest of the project
| const workspacesPckJsonData = await findWorkspacesPackages( | |
| const workspacesPkgJsonData = await findWorkspacesPackages( |
|
@Brianzchen I've added some tests |
|
@Brianzchen just added support for your workspaces |
|
@moroine can you fix the conflicts |
|
@Brianzchen updated |
|
@gantoine any update on this? |
|
@moroine until the changes merge I've published them here https://www.npmjs.com/package/ftyped |
|
@moroine do you mind pulling from master, your test will have a single flow error that you can suppress that's caused by new method-unbinding |
|
@Brianzchen updated |
There was a problem hiding this comment.
So after discussing it with @Brianzchen, I'm happy with this change. Couple things to cleanup:
- Could you fix the tests so they pass on both node versions?
- Can we print warning messages for duplicate versions?
Once those are addressed, I'd want to release this as an RC, so people can try it in their existing yarn monorepos.
|
@gantoine I've updated the PR |
installcommand supports mono-repo