-
Notifications
You must be signed in to change notification settings - Fork 680
build: setup monorepo workspace for dependent packages #2478
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
base: main
Are you sure you want to change the base?
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2478 +/- ##
==========================================
- Coverage 93.09% 93.09% -0.01%
==========================================
Files 40 40
Lines 11240 11239 -1
Branches 713 713
==========================================
- Hits 10464 10463 -1
Misses 764 764
Partials 12 12
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
zimeg
left a comment
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.
📬 A few messages for the kind reviewers!
| - name: Build packages | ||
| run: | | ||
| # Build packages without internal dependencies | ||
| npm run build --workspace=@slack/cli-hooks | ||
| npm run build --workspace=@slack/cli-test | ||
|
|
||
| # Build base dependencies | ||
| npm run build --workspace=@slack/logger | ||
| npm run build --workspace=@slack/types | ||
|
|
||
| # Build packages requiring base dependencies | ||
| npm run build --workspace=@slack/web-api | ||
| npm run build --workspace=@slack/webhook | ||
|
|
||
| # Build packages that depend on the Web API | ||
| npm run build --workspace=@slack/oauth | ||
| npm run build --workspace=@slack/rtm-api | ||
| npm run build --workspace=@slack/socket-mode |
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.
🗣️ note: #2482 is an example of changing dependencies between packages being resolved as expected!
In that example, changes to @slack/types are required in @slack/web-api which cause tests on matching commit f2ac218 to fail in an unchanged branch. We might hope to include these changes as part of related PRs for confident CI without updates across prereleases 🫡
|
|
||
| The script places the reference markdown files in `/docs/english/reference/package-name`. | ||
|
|
||
| ### 🚀 Releases |
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.
🔮 note: These steps are verbose now but I'm hoping to fast follow this change with some enhancement to the release process! For now these steps still package as expected, testing with the following:
$ npm pack --workspace=@slack/web-api --dry-run
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.
📠 note: Apologies for noise in these changes, but a lot of formatting happened I fear.
| "url": "https://github.com/slackapi/node-slack-sdk/issues" | ||
| }, | ||
| "scripts": { | ||
| "prepare": "npm run build", |
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.
🗣️ note: The prepare script is replaced with prepack in all packages!
This helps prevent installation and build errors between packages on an initial install. Otherwise, packages are built when dependencies are installed with the prepare script, and the oauth package requires web-api is built but these are packaged in alphabetical order. Overrides for this aren't obvious to me...
AFAICT this isn't a breaking change since the prepack script is run still when packages are installed from Git. FWIW I cannot figure out how to install packages in a monorepo with that method in both cases either 🤖
| "scripts": { | ||
| "lint": "npx @biomejs/biome check packages", | ||
| "lint:fix": "npx @biomejs/biome check --write packages" | ||
| }, |
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.
🪵 note: Linting becomes a global task with impressive speeds of biome - less than 1 second for all packages.
This is a noticed change to the maintenance tasks I think, but --workspace commands are preferred for package scripts overall, which are run from the root:
$ npm run test --workspace=@slack/webhook
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.
🌲 note: I'm so open to adding more scripts here too, but hope to keep this PR scoped to minimal improvements of workspaces to start:
- Automatic dependent and deduplicated package resolution in development
- Shared linting scripts
- Test fixes and simplified linking related to dependencies
Summary
This PR sets up
workspacesfor improved package management.Requirements