Skip to content

Comments

feat: add support for work objects#2652

Merged
vegeris merged 16 commits intomainfrom
feat-work-objects
Oct 28, 2025
Merged

feat: add support for work objects#2652
vegeris merged 16 commits intomainfrom
feat-work-objects

Conversation

@vegeris
Copy link
Contributor

@vegeris vegeris commented Sep 12, 2025 •
edited
Loading

Summary

This PR pulls in the types and web-api node-slack-sdk packages and updates the view_submission event properties

Requirements (place an x in each [ ])

@vegeris vegeris marked this pull request as draft September 12, 2025 17:26
@codecov
Copy link

codecov bot commented Sep 12, 2025 •
edited
Loading

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.44%. Comparing base (509d6f6) to head (ed533ba).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files 
@@           Coverage Diff           @@
##             main    #2652   +/-   ##
=======================================
  Coverage   93.44%   93.44%           
=======================================
  Files          37       37           
  Lines        7668     7674    +6     
  Branches      669      669           
=======================================
+ Hits         7165     7171    +6     
  Misses        498      498           
  Partials        5        5

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

vegeris
vegeris commented Oct 16, 2025
View reviewed changes
@vegeris vegeris changed the title Do not merge: Add support for work objects Add support for work objects Oct 16, 2025
@vegeris vegeris changed the title Add support for work objects feat: add support for work objects Oct 16, 2025
@vegeris vegeris marked this pull request as ready for review October 16, 2025 01:16
WilliamBergamin
WilliamBergamin previously requested changes Oct 24, 2025
View reviewed changes
Copy link
Contributor

@WilliamBergamin WilliamBergamin left a comment

Choose a reason for hiding this comment

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

These changes look good 💯

I'm just skeptical about including the feature in examples/socket-mode/app.js, I left a comment about that

examples/socket-mode/app.js Outdated

await say(`${command.text}`);
});

Copy link
Contributor

@WilliamBergamin WilliamBergamin Oct 24, 2025

Choose a reason for hiding this comment

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

I don't think this is the best place for this 😅

This example showcases the most used features of slack, for example it doesn't include custom steps or link unfurls so I don't think we should put work objects here

A dedicated sample in https://github.com/slack-samples may be a better place for this 🤔
I also heard that @zimeg may be working on a repository in https://github.com/slack-samples where we can showcase all of slacks features, but I'm not sure if there is a timeline for this

Copy link
Contributor Author

@vegeris vegeris Oct 27, 2025

Choose a reason for hiding this comment

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

Fair! I'll remove it from this PR. Maybe we can just show sample code snippets in the docs for now

Copy link
Member

@zimeg zimeg Oct 28, 2025

Choose a reason for hiding this comment

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

😳 Apologies for missing this note! I'm hoping soon we have collections of focused examples to reference these implementations in code and documentation elsewhere.

I'll keep a note of commit 0657cab since this remains nice code!

Copy link
Contributor Author

@vegeris vegeris Oct 28, 2025

Choose a reason for hiding this comment

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

I'm hoping soon we have collections of focused examples to reference these implementations in code and documentation elsewhere

Yeah, once this is released I'm going to add code snippets to the docs. Separately, it might still be nice to add work objects to a sample app somewhere

examples/socket-mode/README.md
```
// can get this from OAuth & Permission page in app configuration
export BOT_TOKEN=YOUR_SLACK_BOT_TOKEN
export SLACK_BOT_TOKEN=YOUR_SLACK_BOT_TOKEN
Copy link
Contributor

@WilliamBergamin WilliamBergamin Oct 24, 2025

Choose a reason for hiding this comment

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

Praise 🚀 definitely keep this!!

Copy link
Member

@zimeg zimeg Oct 28, 2025

Choose a reason for hiding this comment

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

🌟 praise: Incredible fix! Thanks for finding it too.

src/types/view/index.ts
Comment on lines 316 to 320
entity_url?: string;
external_ref?: { id: string; type?: string };
app_unfurl_url?: string;
message_ts?: string;
channel?: string;
Copy link
Contributor

@WilliamBergamin WilliamBergamin Oct 24, 2025

Choose a reason for hiding this comment

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

I think there might be some type tests in this project, but I'm not sure they cover this interface, if they do they might be worth updating, else this looks good to me 💯

Copy link
Contributor Author

@vegeris vegeris Oct 27, 2025

Choose a reason for hiding this comment

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

I see these tests in test/types/view.test-d.ts but they don't validate specific properties in ViewOutput 👀

// view_submission
app.view('modal-id', async ({ body, view }) => {
  // TODO: the body can be more specific (ViewSubmitAction) here
  expectType<SlackViewAction>(body);
  expectType<ViewOutput>(view);
  // TODO: assert on type assignability for `ack`
});

@vegeris vegeris requested a review from zimeg October 27, 2025 14:17
zimeg
zimeg approved these changes Oct 28, 2025
View reviewed changes
Copy link
Member

@zimeg zimeg left a comment

Choose a reason for hiding this comment

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

@vegeris LGTM! These are nice finds and great additions! 🧰 ✨

examples/socket-mode/README.md
```
// can get this from OAuth & Permission page in app configuration
export BOT_TOKEN=YOUR_SLACK_BOT_TOKEN
export SLACK_BOT_TOKEN=YOUR_SLACK_BOT_TOKEN
Copy link
Member

@zimeg zimeg Oct 28, 2025

Choose a reason for hiding this comment

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

🌟 praise: Incredible fix! Thanks for finding it too.

@zimeg zimeg added enhancement M-T: A feature request for new functionality semver:minor TypeScript-specific labels Oct 28, 2025
@zimeg zimeg added this to the 4.6.0 milestone Oct 28, 2025
@vegeris vegeris dismissed WilliamBergamin’s stale review October 28, 2025 01:12

Changes applied but reviewer is OOO

@vegeris vegeris merged commit 3c0ca6a into main Oct 28, 2025
19 checks passed
@vegeris vegeris deleted the feat-work-objects branch October 28, 2025 15:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@zimeg zimeg zimeg approved these changes

@WilliamBergamin WilliamBergamin Awaiting requested review from WilliamBergamin

Assignees

No one assigned

Labels

enhancement M-T: A feature request for new functionality semver:minor TypeScript-specific

Projects

None yet

Milestone

4.6.0

Development

Successfully merging this pull request may close these issues.

3 participants

@vegeris @zimeg @WilliamBergamin