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: makes types fn and config and key more strictly #946

Merged
merged 1 commit into from
Feb 9, 2021

Conversation

koba04
Copy link
Collaborator

@koba04 koba04 commented Feb 2, 2021

This is a PR to make internal types (fn, config, and key) more strictly.
This helps refactoring and code completion.

In the future, it would be nice if SWR could enable the TypeScript's strict mode.

@codesandbox-ci
Copy link

codesandbox-ci bot commented Feb 2, 2021

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 8d15462:

Sandbox Source
SWR-Basic Configuration
SWR-States Configuration
SWR-Infinite Configuration

// `null` is used for a hack to manage shared state with SWR
// https://github.com/vercel/swr/pull/918
export type fetcherFn<Data> = ((...args: any) => Data | Promise<Data>) | null
export type fetcherFn<Data> = (...args: any) => Data | Promise<Data>
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

null is only accepted for useSWR and useSWRInfinite doesn't accept null as the fetcher, so I've moved null to the argument of useSWR.

export interface ConfigInterface<
Data = any,
Error = any,
Fn extends fetcherFn<Data> = fetcherFn<Data>
> {
errorRetryInterval?: number
errorRetryInterval: number
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

These values are having default values as defaultConfig, so we don't have to make the types optional.

): SWRInfiniteResponseInterface<Data, Error>
function useSWRInfinite<Data = any, Error = any>(
...args
getKey: KeyLoader<Data>,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

getKey is not an optional argument.

config?: ConfigInterface<Data, Error>
// `null` is used for a hack to manage shared state with SWR
// https://github.com/vercel/swr/pull/918
fn?: fetcherFn<Data> | null,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

null should be accepted

@promer94
Copy link
Collaborator

promer94 commented Feb 4, 2021

FYI. I had enabled the strict mode of typescript in the next branch. 😁

@koba04
Copy link
Collaborator Author

koba04 commented Feb 4, 2021

@promer94
Wow, that's so amazing!!!
Do you have any plan to merge the next branch into the master branch? What is the goal for the branch? v1?

@shuding
Copy link
Member

shuding commented Feb 9, 2021

@koba04 We are aiming to v1 and we put all the things that might break the compatibility into that branch. But if you think that strict mode is safe to be added earlier, we can cherry-pick it and get it on master :)

Copy link
Member

@shuding shuding left a comment

Choose a reason for hiding this comment

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

Thanks!

@shuding shuding merged commit 435604c into vercel:master Feb 9, 2021
@koba04 koba04 deleted the strict-types-for-fn-and-config branch February 10, 2021 14:55
@koba04
Copy link
Collaborator Author

koba04 commented Feb 10, 2021

@shuding Thank you!
Do you have any plan to merge the next branch into the master branch? I've found a milestone for v1, which means all issues in the milestone have to be fixed to release v1, don't they?
https://github.com/vercel/swr/issues?q=is%3Aopen+is%3Aissue+milestone%3A1.0

This was referenced Mar 15, 2021
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.

3 participants