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

fix: makeEventFunctions take an array of parameters #2186

Merged
merged 4 commits into from
Aug 20, 2024

Conversation

mofojed
Copy link
Member

@mofojed mofojed commented Aug 12, 2024

  • Previously every emit required exactly one parameter, which is not the case
  • Allow zero or more parameters

- Previously every emit required exactly one parameter, which is not the case
- Allow zero or more parameters
@mofojed mofojed requested a review from mattrunyon August 12, 2024 19:56
@mofojed mofojed self-assigned this Aug 12, 2024
Copy link

codecov bot commented Aug 12, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 46.65%. Comparing base (a257296) to head (61f0c15).
Report is 6 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2186      +/-   ##
==========================================
- Coverage   46.66%   46.65%   -0.02%     
==========================================
  Files         692      692              
  Lines       38629    38583      -46     
  Branches     9814     9845      +31     
==========================================
- Hits        18028    18000      -28     
+ Misses      20548    20530      -18     
  Partials       53       53              
Flag Coverage Δ
unit 46.65% <100.00%> (-0.02%) ⬇️

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

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

Copy link
Collaborator

@mattrunyon mattrunyon left a comment

Choose a reason for hiding this comment

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

Not entirely sure where you'd want to put it or how to incorporate it, but you can use conditional types to support a non-array type as a single param, array/tuple

type MyFunc<P = []> = (...params: P extends unknown[] ? P : [P]) => void;

Also an interesting thing I saw was you can label tuples for argument signatures. See here for more info, but the gist is you could do makeEventFunctions<[count: number, title: string]>(...) and TS should then show the params as count and title in autocomplete instead of arg0 and arg1

- Name the params tuple
- Allow passing in a param list or a single value
@mofojed mofojed requested a review from mattrunyon August 20, 2024 14:20
mattrunyon
mattrunyon previously approved these changes Aug 20, 2024
packages/golden-layout/src/utils/EventUtils.ts Outdated Show resolved Hide resolved
- Added more tests around different payload types as well
@mofojed mofojed merged commit f5b01fd into deephaven:main Aug 20, 2024
11 checks passed
@mofojed mofojed deleted the make-event-functions-parameters branch August 20, 2024 20:12
@github-actions github-actions bot locked and limited conversation to collaborators Aug 20, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants