Skip to content
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.

Sign up for GitHub

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

ci: test samples and examples with a packaged bolt build #2372

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

ci: test samples and examples with a packaged bolt build #2372

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
3fba83d
ci: attempt to test typescript samples without dev deps
zimeg Dec 17, 2024
3d8f5a3
ci: package the changes instead of linking a checkout
zimeg Dec 17, 2024
ab2504f
fix: jump two directories up to the packaged tgz
zimeg Dec 17, 2024
e5ad367
ci: perform typechecking on the oauth express receiver js app
zimeg Dec 17, 2024
155d38b
test: add a type statement that requires unmet dependencies
zimeg Dec 17, 2024
5ea64ca
ci: remove bolt-js dependencies before testing apps
zimeg Dec 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 9 additions & 9 deletions .github/workflows/samples.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,17 +15,18 @@ jobs:
example:
- examples/getting-started-typescript
- examples/custom-receiver
- examples/oauth-express-receiver
steps:
- name: Use Node.js ${{ matrix.node-version }}
uses: actions/setup-node@v4
with:
node-version: ${{ matrix.node-version }}
- uses: actions/checkout@v4
- name: Install dependencies and link bolt-js
run: npm i && npm link .
- name: Install example dependencies and link bolt-js
- name: Package the latest changes to bolt-js
run: npm i && npm pack . && rm -rf node_modules
- name: Install example dependencies for testing
working-directory: ${{ matrix.example }}
run: npm i && npm link @slack/bolt
run: npm i && npm i ../../slack-bolt-*.tgz
Comment on lines +25 to +29
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some TILs are included here, though I believe this can still be improved:

  • npm pack will package @slack/bolt as if it's going to be released
  • node_modules of the bolt-js repo must be removed or nested directories can make use of these pacakges

The *.tgz file seems ideal for testing in a production environment like this since it lets us build bolt-js then remove its node_modules before testing a sample!

I'm not thrilled with the relative paths here, but am interested in if this is an approach we're alright taking?

- name: Compile sample
working-directory: ${{ matrix.example }}
run: npm run build
Expand All @@ -46,18 +47,17 @@ jobs:
uses: actions/checkout@v4
with:
path: ./bolt-js
- name: Install and link bolt-js
- name: Package the latest changes to bolt-js
working-directory: ./bolt-js
run: npm i && npm link .
run: npm i && npm pack . && rm -rf node_modules
- name: Checkout ${{ matrix.sample }}
uses: actions/checkout@v4
with:
repository: ${{ matrix.sample }}
path: ./sample
- name: Install sample dependencies and link bolt-js
- name: Install sample dependencies for testing
working-directory: ./sample
run: npm i && npm link @slack/bolt
run: npm i && npm i ../bolt-js/slack-bolt-*.tgz
- name: Compile sample
working-directory: ./sample
run: npx tsc

5 changes: 5 additions & 0 deletions examples/oauth-express-receiver/app.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
const { App, ExpressReceiver, LogLevel, FileInstallationStore } = require('@slack/bolt');

/**
* @type {import("@slack/bolt/dist/receivers/ParamsIncomingMessage").ParamsIncomingMessage};
*/
const _example = null;
Comment on lines +3 to +6
Copy link
Member Author

@zimeg zimeg Dec 17, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TBH I think ParamsIncomingMessage should be exported at the top-level of @slack/bolt since it describes the "req" parameter of receivers, but that's unchanged in this PR.

However I'm super curious to know if exporting it from the top-level would fix the current typescript error here 🤔


// Create an ExpressReceiver
const receiver = new ExpressReceiver({
signingSecret: process.env.SLACK_SIGNING_SECRET,
Expand Down
5 changes: 4 additions & 1 deletion examples/oauth-express-receiver/package.json
View file
Open in desktop
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updating examples continues to be an upcoming task, but I found this app to be helpful for testing the current edges.

FWIW I'm interested in knowing if we want to include jsdoc as part of examples for typechecking like this or not? That can of course be for a follow up, but I think it might be neat 🔍

Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,14 @@
"main": "app.js",
"scripts": {
"start": "node app.js",
"test": "echo \"Error: no test specified\" && exit 1"
"build": "npx tsc --allowJs --noEmit --esModuleInterop app.js"
},
"author": "Slack Technologies, Inc.",
"license": "MIT",
"dependencies": {
"@slack/bolt": "^3.6.0"
},
"devDependencies": {
"typescript": "^5.7.2"
}
}
Loading