Skip to content

Make url optional. Remove krustykrab from dependencies for react-select-fetch#191

Open
soteri-scottmmjackson wants to merge 1 commit intovtaits:masterfrom
soteri-scottmmjackson:sj-optional-url-and-kill-krustykrab
Open

Make url optional. Remove krustykrab from dependencies for react-select-fetch#191
soteri-scottmmjackson wants to merge 1 commit intovtaits:masterfrom
soteri-scottmmjackson:sj-optional-url-and-kill-krustykrab

Conversation

@soteri-scottmmjackson
Copy link
Copy Markdown

Fixes #190
Fixes #189

}
const paramsStr = stringifyParams(params);

const response: Response = await fetch(`${url}?${paramsStr}`, {
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Looks like it would be better to do url ?? '' instead of throwing an error above.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Is this different from setting url = "" default here?

I'd decided on throwing an error because it was least surprising compared to the previous behavior of throwing an inspection error.

I think I also might have found a minimal change with no associated functional code that would do it:

export type UseSelectFetchMapParams<
	OptionType,
	Group extends GroupBase<OptionType>,
> = ({ url: string, get: never} | {get: Get, url: never}) & {
	queryParams?: {
		[key: string]: unknown;
	};
	searchParamName?: string | null;
	pageParamName?: string | null;
	offsetParamName?: string | null;
	mapResponse?: MapResponse<OptionType, Group>;
	initialPage?: number;
	defaultInitialPage?: number;
};

I think this means we can have url xor get?

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

This library aims to provide a convenient way to work with api endpoints. get is supposed to provide headers to request.

What is your case and why don't you use react-select-async-paginate for it?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants