Skip to content

Comments

web-api(fix):Update SearchMessagesArguments to support cursor pagination#2361

Merged
vegeris merged 3 commits intomainfrom
update-search-messages
Oct 23, 2025
Merged

web-api(fix):Update SearchMessagesArguments to support cursor pagination#2361
vegeris merged 3 commits intomainfrom
update-search-messages

Conversation

@vegeris
Copy link
Contributor

@vegeris vegeris commented Sep 8, 2025

Summary

Requirements ()

@codecov
Copy link

codecov bot commented Sep 8, 2025 •
edited
Loading

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.01%. Comparing base (7c60024) to head (db6bdbb).
⚠️ Report is 1 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files 
@@           Coverage Diff           @@
##             main    #2361   +/-   ##
=======================================
  Coverage   93.01%   93.01%           
=======================================
  Files          40       40           
  Lines       11111    11111           
  Branches      713      713           
=======================================
  Hits        10335    10335           
  Misses        764      764           
  Partials       12       12
Flag Coverage Δ
cli-hooks 95.23% <ø> (ø)
cli-test 94.79% <ø> (ø)
oauth 77.39% <ø> (ø)
socket-mode 61.87% <ø> (ø)
web-api 98.07% <ø> (ø)
webhook 96.66% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@vegeris vegeris force-pushed the update-search-messages branch from 52ed286 to 7350c81 Compare September 8, 2025 15:40
@zimeg zimeg linked an issue Sep 8, 2025 that may be closed by this pull request
Support cursor pagination in search.messages API calls #2360
Closed
7 tasks
@zimeg zimeg added bug M-T: A confirmed bug report. Issues are confirmed when the reproduction steps are documented semver:patch pkg:web-api applies to `@slack/web-api` labels Sep 9, 2025
@zimeg zimeg added this to the [email protected] milestone Sep 9, 2025
zimeg
zimeg approved these changes Sep 9, 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! This seems good to merge when the time is right but it might be nice to add a type test for this before? 🧪

node-slack-sdk/packages/web-api/test/types/methods/search.test-d.ts

Lines 28 to 37 in 893a37d

// search.messages
// -- sad path
expectError(web.search.messages()); // lacking argument
expectError(web.search.messages({})); // empty argument
// -- happy path
expectAssignable<Parameters<typeof web.search.messages>>([
{
query: '1234', // must specify query
},
]);

I left a few other comments too!

packages/web-api/src/types/request/search.ts
Comment on lines +17 to +19
* @description Paginate through collections of data by setting the `cursor` parameter to `*` for the first "page"
* or a `next_cursor` attribute returned by a previous request's `response_metadata`.
* Use the `count` parameter to set the number of items to return per page rather than `limit`.
Copy link
Member

@zimeg zimeg Sep 9, 2025

Choose a reason for hiding this comment

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

🧮 praise: Thank you for including this.

packages/web-api/src/types/request/search.ts
extends TokenOverridable,
TraditionalPagingEnabled,
Searchable,
SearchMessagesCursorPagination {}
Copy link
Member

@zimeg zimeg Sep 9, 2025

Choose a reason for hiding this comment

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

👁️‍🗨️ suggestion(non-blocking): I don't believe a similar cursor is used elsewhere? Might be nice to inline this IMO but no blocker!

Copy link
Contributor Author

@vegeris vegeris Oct 23, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

This is how Biome wants it to look 😛

Yeah, I think an API-specific interface makes sense here. The same count param from the TraditionalPagingEnabled interface is reused for cursor pagination

@vegeris vegeris force-pushed the update-search-messages branch from f8936ad to db6bdbb Compare October 23, 2025 20:16
@vegeris vegeris merged commit 16bd88e into main Oct 23, 2025
57 checks passed
@vegeris vegeris deleted the update-search-messages branch October 23, 2025 20:24
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

Assignees

No one assigned

Labels

bug M-T: A confirmed bug report. Issues are confirmed when the reproduction steps are documented pkg:web-api applies to `@slack/web-api` semver:patch

Projects

None yet

Milestone

[email protected]

Development

Successfully merging this pull request may close these issues.

Support cursor pagination in search.messages API calls

2 participants

@vegeris @zimeg