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.

Sign up for GitHub

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

Jump to bottom

StoreName type check not depends on infer #10

Merged
merged 6 commits into from
Sep 13, 2023
Merged

StoreName type check not depends on infer #10

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
691cb76
StoreName type check not depends on infer
AkifumiSato Sep 12, 2023
ce4b2f7
rename DefaultStoreName to DefaultStoreNames
AkifumiSato Sep 12, 2023
e6b98a1
fix hooks's StoreName default type
AkifumiSato Sep 12, 2023
5558c02
refactor internal hooks type
AkifumiSato Sep 12, 2023
ffc4b8a
add getHooksWith<StoreNames>
AkifumiSato Sep 13, 2023
f5eb191
refactor StoreNames to StoreName, delete redundant descriptions.
AkifumiSato Sep 13, 2023
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
8 changes: 6 additions & 2 deletions apps/example-next/src/components/Counter.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
"use client";

import { useLocationState, StoreName, Refine } from "@location-state/core";
import {
useLocationState,
DefaultStoreNames,
Refine,
} from "@location-state/core";
import { z, ZodType } from "zod";

const zodRefine =
Expand All @@ -10,7 +14,7 @@ const zodRefine =
return result.success ? result.data : undefined;
};

export function Counter({ storeName }: { storeName: StoreName }) {
export function Counter({ storeName }: { storeName: DefaultStoreNames }) {
const [counter, setCounter] = useLocationState({
name: "counter",
defaultValue: 0,
Expand Down
4 changes: 2 additions & 2 deletions apps/example-next/src/components/List.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
"use client";

import { useLocationState, StoreName } from "@location-state/core";
import { useLocationState, DefaultStoreNames } from "@location-state/core";

export function List({ storeName }: { storeName: StoreName }) {
export function List({ storeName }: { storeName: DefaultStoreNames }) {
const [displayList, setDisplayList] = useLocationState({
name: "display-list",
defaultValue: false,
Expand Down
51 changes: 39 additions & 12 deletions packages/location-state-core/src/hooks.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,21 +1,25 @@
import { LocationStateContext } from "./context";
import { StoreName } from "./types";
import { DefaultStoreNames } from "./types";
import { useCallback, useContext, useState, useSyncExternalStore } from "react";

export type Refine<T> = (value: unknown) => T | undefined;

export type LocationStateDefinition<T> = {
export type LocationStateDefinition<
T,
StoreName extends string = DefaultStoreNames,
> = {
name: string;
defaultValue: T;
storeName: StoreName | string;
// Avoid inference to rigorously check type arguments
storeName: StoreName extends infer S ? S : never;
Copy link
Member

Choose a reason for hiding this comment

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

これがうまくいくにはStoreNameのデフォルトがQiitaの記事のようにneverでないとダメなのでは？
string extends infer SだとSはstringをキャプチャしてしまうからneverにはならないと思われ

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

LocationStateDefinition 単体ではこれで推論されることはなくなる＋明示的に型引数で指定するとうまくいってました。
Qiitaの記事は型引数を必ず指定することを目的として推論させない＋neverデフォルトなアプローチであって、推論されることを防ぐのみならStoreName extends infer S ? S : never;でOKだからうまくいったんだと思ってました。

Copy link
Member

Choose a reason for hiding this comment

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

LocationStateDefinition単体だとstoreName: StoreNameでもエラーになっていたはず
つまり型変数StoreNameが普通にDefaultStoreName(s)になるんでしょう
関数の型推論が特別に強力なんだろうなぁ

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

あ、確かにでした。
関数の推論をStoreName extends infer S ? S : never;で防げるからうまくいくってところですかね...？

Copy link
Member

Choose a reason for hiding this comment

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

この修正もう不要ですよね？
storeName: StoreNameでいいはず

refine?: Refine<T>;
};

type Updater<T> = (prev: T) => T;
type UpdaterOrValue<T> = T | Updater<T>;
type SetState<T> = (updaterOrValue: UpdaterOrValue<T>) => void;

const useStore = (storeName: StoreName | string) => {
const useStore = (storeName: string) => {
const { stores } = useContext(LocationStateContext);
const store = stores[storeName];
if (!store) {
Expand All @@ -25,16 +29,19 @@ const useStore = (storeName: StoreName | string) => {
return store;
};

export const useLocationState = <T>(
definition: LocationStateDefinition<T>,
const _useLocationState = <T, StoreName extends string = DefaultStoreNames>(
Copy link
Member

Choose a reason for hiding this comment

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

型引数のデフォルトも不要かなと

definition: LocationStateDefinition<T, StoreName>,
): [T, SetState<T>] => {
const storeState = useLocationStateValue(definition);
const setStoreState = useLocationSetState(definition);
const storeState = _useLocationStateValue(definition);
const setStoreState = _useLocationSetState(definition);
return [storeState, setStoreState];
};

export const useLocationStateValue = <T>(
definition: LocationStateDefinition<T>,
const _useLocationStateValue = <
T,
StoreName extends string = DefaultStoreNames,
Copy link
Member

Choose a reason for hiding this comment

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

同上

>(
definition: LocationStateDefinition<T, StoreName>,
): T => {
const { name, defaultValue, storeName, refine } = useState(definition)[0];
const store = useStore(storeName);
Expand All @@ -56,8 +63,8 @@ export const useLocationStateValue = <T>(
return storeState;
};

export const useLocationSetState = <T>(
definition: LocationStateDefinition<T>,
const _useLocationSetState = <T, StoreName extends string = DefaultStoreNames>(
Copy link
Member

Choose a reason for hiding this comment

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

同上

definition: LocationStateDefinition<T, StoreName>,
): SetState<T> => {
const { name, defaultValue, storeName, refine } = useState(definition)[0];
const store = useStore(storeName);
Expand All @@ -78,3 +85,23 @@ export const useLocationSetState = <T>(
);
return setStoreState;
};

export const getHooksWith = <StoreNames extends string>() =>
Copy link
Member

Choose a reason for hiding this comment

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

自分のせいだけどStoreNamesとStoreNameで一貫性がなくなってますね。。。
DefaultStoreNamesも含めてどちらかに揃えたい
元々はデフォルトのストア (2つあるから複数) の名前なので複数形がいいかなと思ってDefaultStoreNamesにしてもらったんだけど、このファイルの型引数の場合は単独のストア名の型だから単数形にするべきですよねぇ
そうするとDefaultStoreNamesも単数形にすべきか。。。
なんかそんな気がしてきたので単数形に揃えますか
手間を増やして申し訳ない

({
useLocationState: _useLocationState,
useLocationStateValue: _useLocationStateValue,
useLocationSetState: _useLocationSetState,
} as {
useLocationState: <T>(
definition: LocationStateDefinition<T, StoreNames>,
) => [T, SetState<T>];
useLocationStateValue: <T>(
definition: LocationStateDefinition<T, StoreNames>,
) => T;
useLocationSetState: <T>(
definition: LocationStateDefinition<T, StoreNames>,
) => SetState<T>;
});

export const { useLocationState, useLocationStateValue, useLocationSetState } =
getHooksWith<DefaultStoreNames>();
2 changes: 1 addition & 1 deletion packages/location-state-core/src/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/// <reference types="navigation-api-types" />

export type StoreName = "session" | "url";
export type DefaultStoreNames = "session" | "url";
Copy link
Member

Choose a reason for hiding this comment

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

ということでDefaultStoreNameで


export type Syncer = {
key(): string | undefined;
Expand Down