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.

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

types 3.0: breaking changes for block kit types #1905

Open
1 task
filmaj opened this issue Aug 29, 2024 · 3 comments
Open
1 task

types 3.0: breaking changes for block kit types #1905

filmaj opened this issue Aug 29, 2024 · 3 comments
Assignees
Labels
area:typescript issues that specifically impact using the package from typescript projects enhancement M-T: A feature request for new functionality pkg:types applies to `@slack/types` semver:major

Comments

@filmaj
Copy link
Contributor

filmaj commented Aug 29, 2024

Taking inspiration from @seratch's library, https://github.com/seratch/slack-web-api-client/tree/main/src/block-kit, should study the following to see what kinds of improvements on the Block Kit side we could leverage in @slack/types.

Also, we should study bolt-js' use of composable event payloads and recursive Block Kit types, to see what improvements could land here. See the issues tagged in #1904

Idea: Add generics to layout blocks

What if we extended Blocks with generics, so that a containing layout block could constrain or fully specify the block elements contained within. An example on how this would look like for a dev:

const myImage: ImageElement = {
  type: 'image',
  alt_text: 'kitteh',
  image_url: 'https://kittens-r-us.com/kitteh.jpg',
}
const myContextBlock: ContextBlock<ImageElement> = {
  type: 'context',
  elements: [myImage], // compiles fine
}
const nopeBlock: ContextBlock<ImageElement> = {
  type: 'context',
  elements: [{ type: 'text', text: 'hello' }], // nope, TS complains that { type: 'text' } not assignable to ImageElement
}

The benefit here is more control and composability. Extending this further, we could employ a similar approach to other Slack domain objects and event payloads, so that they could all be composed in a similar way. For example, a block_actions event payload from a button click could be modeled by passing the ButtonElement into the BlockActionsEvent via generic (BlockActionsEvent<ButtonElement>), or the same idea for composing a View, and then passing that View into a ViewSubmissionEvent.. and so on.

Other Changes

@filmaj filmaj added semver:major enhancement M-T: A feature request for new functionality area:typescript issues that specifically impact using the package from typescript projects pkg:types applies to `@slack/types` labels Aug 29, 2024
@filmaj filmaj added this to the [email protected] milestone Aug 29, 2024
@filmaj filmaj self-assigned this Aug 29, 2024
@seratch
Copy link
Member

seratch commented Sep 2, 2024

Nice!

ContextBlock

This may not work out because a context object can a few different elements at a time. Thus, just making the elements property type more specific would be a reasonable improvement: https://github.com/seratch/slack-web-api-client/blob/1.0.3/src/block-kit/blocks.ts#L95

Aside from that, we may want to give more attention to both block types and block element types, that are acceptable under a condition. More specificallty, message data accepts these blocks while home tab and modal data are different: https://github.com/seratch/slack-web-api-client/blob/1.0.3/src/block-kit/blocks.ts#L39-L71

@slack/web-api currently accepts any blocks for chat.postMessage API in TS, but we can make it more specific this way.

Even if we introduce the type changes in the next major version, most currently working code should not have any runtime issues. Thus, I agree that this change is worth considering for better developer experience in the long run. The only pain points developers would have for the migration would be udpating explicitly set types on the user side (e.g., having KnownBlock[]; within a developers' code).

@filmaj
Copy link
Contributor Author

filmaj commented Sep 3, 2024

Thanks for commenting @seratch ! I was hoping to generate some discussion to test some of my assumptions and ideas and see if they could work or not, if they are useful or not.

This may not work out because a context object can a few different elements at a time.

I should be more specific / detailed in my proposal 😅 . What prompted this idea of using generics for layout blocks was trying to better model the the "Legacy Fields" of Secondary Attachments. In particular, the author_icon field accepts a Context Block with an Image Element. As you pointed out @seratch, by default the Context Block accepts arrays of both image and text elements - but not author_icon! It is a bit more constrained (images only).

Let me expand my pseudocode example; what if by default the ContextBlock generic I described in my original post assigned the generic to be a union of Image and Text elements (meeting the default constraints for Context), but allowed overwriting it to constrain it even more in certain situations (like author_icon in secondary attachments)? E.g.:

type ContextElements = ImageElement | TextElement;

// by default, either text or image elements can be assigned to ContextBlock
interface ContextBlock<Elements extends ContextElements = ContextElements> {
  elements: Elements[];
}
// .. but we can override it in more constrained situations, like `author_icon` in Secondary Attachments:
interface Attachment {
  author_icon: ContextBlock<ImageElement>;
}

This way the 'standard' use of ContextBlock would be satisfied, but more constrained versions could also be defined, too.

Aside from that, we may want to give more attention to both block types and block element types, that are acceptable under a condition.

I like this; it seems to mirror the "Available in surfaces" column of the layout block breakdown on our documentation.

@slack/web-api currently accepts any blocks for chat.postMessage API in TS, but we can make it more specific this way.

Love this!

The only pain points developers would have for the migration would be updating explicitly set types on the user side

Indeed, but as part of any major version release, I plan on writing a migration guide for the most common breaking change scenarios, and this scenario would be covered in that document.

@filmaj
Copy link
Contributor Author

filmaj commented Sep 3, 2024

I've added "define utility types for availability of blocks in different Slack surfaces" as a task on this issue, but technically this could be a new utility type exposed in v2.x of types - a minor change, not a major one. Perhaps worth addressing that earlier and releasing in types v2.x - what do you think?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:typescript issues that specifically impact using the package from typescript projects enhancement M-T: A feature request for new functionality pkg:types applies to `@slack/types` semver:major
Projects
None yet
Development

No branches or pull requests

2 participants