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

feat: add toast types #281

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/types/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { TextStyle, TouchableOpacityProps, ViewStyle } from 'react-native';

export type ReactChildren = React.ReactNode;

export type ToastType = string;
export type ToastType = 'success' | 'error' | 'info' | string;
Copy link
Owner

Choose a reason for hiding this comment

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

Hi @ShivamJoker, unfortunately this does not help: adding | string at the end widens the type of ToastType to string.

Copy link
Author

Choose a reason for hiding this comment

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

@calintamas there should be some way to have all the types ?

Copy link
Owner

Choose a reason for hiding this comment

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

I suppose it should dynamically extract all the possible types from the config prop

Copy link
Author

Choose a reason for hiding this comment

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

No it doesn't give any auto completion

Copy link
Owner

Choose a reason for hiding this comment

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

Indeed, it's implemented as a simple string at this point. I was just suggesting how I think it can be improved

Copy link
Owner

@calintamas calintamas Nov 15, 2021

Choose a reason for hiding this comment

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

You know what I mean?

ToastType to be dynamically computed based on the config prop.
For example:

const toastConfig = {
  foo: () => {},
  bar: () => {},
  baz: () => {}
}

<Toast config={toastConfig} />

Would result in:

export type ToastType = 'success' | 'error' | 'info' | 'foo' | 'bar' | 'baz';

Copy link
Author

Choose a reason for hiding this comment

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

Oh I see! Here is the issue which I was going through to see if we can do anything about it microsoft/TypeScript#29729

I am not sure how will we dynamically generate it, but in redux toolkit they are using some stuffs to do dynamically generate types.

Copy link

Choose a reason for hiding this comment

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

A solution would be to take a generic.

type ToastTypes = 'success' | 'error' | 'notification';

const toastConfig: ToastConfig<ToastTypes> = {
success: () => {},
error: () => {},
notification: () => {}
}

<Toast<ToastTypes> config={toastConfig} />

ToastConfig would then be

type ToastConfig<ToastTypes extends *something to ensure it is a string union*> = {
    [key: ToastTypes]: (params: ToastConfigParams<any>) => React.ReactNode;
};

But this doesn't really solve the Toast.show() autocomplete.

An other way would be to allow users to extends the type of the library as I do with react-navigation:

// Globally type react-navigation
declare global {
  // eslint-disable-next-line @typescript-eslint/no-namespace
  namespace ReactNavigation {
    interface RootParamList extends RoutesParams {}
  }
}

On this example I set the global RootParamList to my RoutesParams so my navigation.navigate() have proper autocomplete.

An other way to do this without global types editing is to hint the user to make a wrapper. They create the following function with the generic I proposed above:

export function toastShow(arg) {
  return Toast.show<ToastTypes>(arg);
}

This wrapper only provide the type, as we could do with useNavigation without editing the global types:

export function useNavigation() {
  return useNavigation<MyNavigationTypes>();
}

export type ToastPosition = 'top' | 'bottom';

export type ToastOptions = {
Expand Down