-
Notifications
You must be signed in to change notification settings - Fork 12
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: support pnpm workspaces #160
Conversation
throw an error in case existing both `pnpm-workspace.yaml` and `package.json#workspaces` fields add `execa` as dependency cleanup unused deps
🦋 Changeset detectedLatest commit: c69d5b6 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
"dependency-graph": "^0.11.0", | ||
"execa": "5.1.1", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
execa
imported and should be present in dependencies
"@rollup/plugin-json": "^6.0.0", | ||
"@rollup/plugin-node-resolve": "^13.3.0", | ||
"@vercel/ncc": "^0.36.0", | ||
"builtins": "^5.0.1", | ||
"consola": "^2.15.3", | ||
"cross-spawn": "^7.0.3", | ||
"dependency-graph": "^0.11.0", | ||
"execa": "5.1.1", | ||
"fs-extra": "^10.1.0", | ||
"globby": "^11.0.0", | ||
"js-yaml": "^4.1.0", | ||
"lodash.get": "^4.4.2", | ||
"minimatch": "^5.1.0", | ||
"mkdirp": "^1.0.4", | ||
"p-limit": "^3.1.0", | ||
"param-case": "^3.0.4", | ||
"resolve.exports": "^1.1.0", | ||
"rollup": "^2.75.6", | ||
"rollup-plugin-generate-package-json": "^3.2.0", | ||
"rollup-plugin-typescript2": "^0.34.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
seems bunch of these dependencies were nowhere used
function compilerOptionsToArgs( | ||
options: Record<string, unknown> | ||
): Array<string> { | ||
const args: Array<string> = []; | ||
for (const [key, value] of Object.entries(options)) { | ||
args.push(`--${key}`, `${value}`); | ||
} | ||
return args; | ||
function compilerOptionsToArgs(options: Record<string, unknown>): string[] { | ||
return Object.entries(options).flatMap(([key, value]) => [ | ||
`--${key}`, | ||
`${value}`, | ||
]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just for better readability
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the former has better readability for me 😁
declare module 'rollup-plugin-generate-package-json'; | ||
declare module 'rollup-plugin-auto-external'; | ||
declare module 'builtins'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
does I was right that they are unused?
const pnpmWorkspacePath = path.join(process.cwd(), "pnpm-workspace.yaml"); | ||
const isPnpmWorkspace = await fse.pathExists(pnpmWorkspacePath); | ||
|
||
if (isPnpmWorkspace) { | ||
if (result) { | ||
throw new Error( | ||
"Both `pnpm-workspace.yaml` and `package.json#workspaces` are not supported. Remove `package.json#workspaces` field." | ||
); | ||
} | ||
|
||
result = jsYaml.load(await fse.readFile(pnpmWorkspacePath, "utf8")) as { | ||
packages?: string[]; | ||
}; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
here magic happens and I think it is better place to validate it in getWorkspaces
than validatePackageJson
type Exports = | ||
| string | ||
| { | ||
require?: string | Record<string, string>; | ||
import?: string | Record<string, string>; | ||
default?: string | Record<string, string>; | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that type was inlined 2 times, I extracted it
{ | ||
"name": "monorepo-pnpm", | ||
"private": true | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this test is identical to simple-monorepo
except that it contain pnpm-workspace.yaml
pnpm-workspace.yaml
pnpm-workspace.yaml
andpackage.json#workspaces
fieldsexeca
as dependency