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

View router discrete methods #81

Merged
merged 12 commits into from
Aug 19, 2022
Merged

Conversation

filmaj
Copy link
Contributor

@filmaj filmaj commented Aug 18, 2022

API / Design

Tip of the 🎩 to @selfcontained for suggesting this API.

export const { viewSubmission, viewClosed } = ViewsRouter(MyFunctionDefinition)
  .addSubmissionHandler("view_1", async ({ inputs, body /* of type ViewSubmissionInvocationBody */, view, env, token }) => { ... })
  .addClosedHandler("view_1", async ({ inputs, body /* of type ViewClosedInvocationBody */, view, env, token }) => { ... })

Things That Do Not Work

While the deno-slack-sdk compiles, userland code has a problem with the export const { viewSubmission, viewClosed } line; it doesn't like the viewSubmission and viewClosed properties, thinks they don't belong there:

error: TS2339 [ERROR]: Property 'viewSubmission' does not exist on type 'ViewRouter<{ requester_id: { type: "slack#/types/user_id"; description: string; }; approval_channel_id: { type: "slack#/types/channel_id"; description: string; }; subject: { type: "string"; description: string; }; details: { ...; }; }, { ...; }, ("requester_id" | ... 2 more ... | "details")[], ("message_ts" | ... 1 ...'.
export const { viewSubmission, viewClosed } = ViewsRouter(ApprovalFunction)
               ~~~~~~~~~~~~~~
    at file:///Users/fmaj/src/interactive-approval/functions/approval/mod.ts:130:16

The above only happens when I attached handlers to the ViewsRouter object via the add* method(s). If I drop those, then the complaint goes away. This tells me something in the return value from the type-asserted exportedHandler duck-punched addSubmissionHandler and addClosedHandler magic code in the ViewsRouter method is off.

☝️This is fixed now - but leaving the explanation for historical purposes. We ended up adjusting how we wrap/export the viewClosed and viewSubmission handlers so they're directly on the ViewsRouter class now.

Things I Do Not Like

I feel like I'm duplicating a lot of code for no reason, and that by using generics and TypeScript "properly" I could avoid that. I tried very much in different ways to do this but all of them failed. This is as close as I've gotten to making this work 😓 😣 😧

Changes

  • Ordering block kit action type properties alphabetically and adding some docs to the properties
  • 🚨 Made Block Kit action payload properties enterprise and is_enterprise_install NOT optional; they seem to always be present, even in non-enterprise workspaces.
  • Renamed the src/functions/routers directory to src/functions/interactivity for clarity

TODO

  • Types for view payloads and type handlers
  • The actual view router code
  • 🐛 Userland code trying to pop the exports into place in function code leads to TS complaints; seems to be some issue with the ViewsRouter() constructor type assertion magic
  • Tests
  • Docs under docs/

@filmaj filmaj requested a review from a team as a code owner August 18, 2022 17:02
@codecov
Copy link

codecov bot commented Aug 18, 2022

Codecov Report

Merging #81 (194ebc6) into main (00bd2ff) will increase coverage by 0.29%.
The diff coverage is 99.38%.

@@            Coverage Diff             @@
##             main      #81      +/-   ##
==========================================
+ Coverage   95.80%   96.09%   +0.29%     
==========================================
  Files          39       41       +2     
  Lines        1431     1564     +133     
  Branches       80       87       +7     
==========================================
+ Hits         1371     1503     +132     
- Misses         58       59       +1     
  Partials        2        2              
Impacted Files Coverage Δ
src/functions/interactivity/view_router.ts 99.20% <99.20%> (ø)
src/functions/interactivity/action_router.ts 100.00% <100.00%> (ø)
src/functions/interactivity/matchers.ts 100.00% <100.00%> (ø)
src/functions/interactivity/mod.ts 100.00% <100.00%> (ø)
src/mod.ts 100.00% <100.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@filmaj filmaj self-assigned this Aug 18, 2022
Copy link
Contributor

@selfcontained selfcontained left a comment

Choose a reason for hiding this comment

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

Nice work, thanks for doing this.

@selfcontained selfcontained merged commit fbc6c5f into main Aug 19, 2022
@selfcontained selfcontained deleted the view-router-discrete-methods branch August 19, 2022 00:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants