-
Notifications
You must be signed in to change notification settings - Fork 10
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
feat: Add basic TypeScript types #26
Conversation
I do not expect these to be completely accurate. However, they should be a decent starting point for further discussion!
export function useRollbarCaptureEvent(metadata: object, level?: LEVEL): void; | ||
export function isValidLevel(): boolean; | ||
|
||
export function historyContext( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am unsure about most of historyContext
, especially filter
, formatter
, and v4x
s
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Improved them but still not 100% confident
index.d.ts
Outdated
children: ReactNode; | ||
fallbackUI?: ReactNode | ||
errorMessage?: string | (() => string) | ||
extra?: Record<string | number, unknown> | (() => Record<string | number, unknown>) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am unsure about extra
It would be very helpful to have types as part of the module. I found one component missing a type when I tried to use this myself - Here's an implementation that worked for me:
For anyone hacking this into their own project manually, you'll want to wrap the index.d.ts from this PR: declare module '@rollbar/react' {
// types go here
} And put it in a place that the types are picked up, e.g. if you have typeRoots in your {
"typeRoots": ["./types/"]
} Then put the file as |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@quinnturner thank you.
Merging this, and welcoming follow up PRs to fix any remaining issues.
@waltjones @calvinf, I added types for |
@quinnturner @calvinf v0.9.0 is released now with both PRs included. |
Description of the change
I do not expect these to be completely accurate.
However, they should be a decent starting point for further discussion!
Type of change
Related issues
Checklists
Development
Code review