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

How To use it in TypeScript #23

Open
Ofer-Gal opened this issue Nov 2, 2021 · 28 comments
Open

How To use it in TypeScript #23

Ofer-Gal opened this issue Nov 2, 2021 · 28 comments
Assignees
Projects
Milestone

Comments

@Ofer-Gal
Copy link

Ofer-Gal commented Nov 2, 2021

What version of this package are you using? 0.5.0

What problem do you want to solve? using TypeScript

My store.ts has

import createStore from "fragstore";
import { IServiceTypes } from "../webparts/ace2StepsRequest/components/IAce2StepsRequestProps";
export const { useStore: useECGProject } = createStore({ upfiles: [], theProject: 'None', requestStep: 1 });
export const { useStore: useDPCConfig } = createStore({ services: [] });

How would I make "services" be of type IServiceTypes ?

@danielart
Copy link
Collaborator

danielart commented Nov 3, 2021

Hi! I think you can create an interface for the initial state, declare the initial state outside in a const, and pass it to createStore. Could you try to type in this way? (not sure if services type should be IServiceTypes or IServiceTypes [])

interface InitialState{
   services: IServiceTypes  
}

const initialState:InitialState = {
  services: []
}

export const { useStore } = createStore(initialState);

@Chetan-unbounce
Copy link

is there a plan to convert the createStore into typescript and accept a generic type for the state

@aralroca
Copy link
Collaborator

aralroca commented Nov 9, 2021

is there a plan to convert the createStore into typescript and accept a generic type for the state

Yes, probably for 0.8 version 😊

@aralroca aralroca added this to the 0.8.0 milestone Nov 9, 2021
@aralroca
Copy link
Collaborator

aralroca commented Nov 9, 2021

For the moment the types are generated by default https://www.runpkg.com/?teaful@0.7.2/dist/index.d.ts

@laekem34
Copy link

laekem34 commented Nov 9, 2021

Hello !
This is a proposal of type definition :

declare module 'teaful' {
	import React from 'react';
	type Hook<S> = (
		initial?: S,
		onAfterUpdate?: afterCallbackType<S>
	) => HookReturn<S>;

	type HookDry<S> = (initial?: S) => HookReturn<S>;

	type HookReturn<T> = [T, (value: T) => void, () => {}];

	export type Hoc<S> = { store: HookReturn<S> };

	type HocFunc<S, R extends React.ComponentClass = React.ComponentClass> = (
		component: R,
		initial?: S
	) => R;

	type useStoreType<S extends initialType> = {
		[k in keyof S]: S[k] extends initialType ? useStoreType<S[k]> : Hook<S[k]>;
	};

	type getStoreType<S extends initialType> = {
		[k in keyof S]: S[k] extends initialType
			? useStoreType<S[k]>
			: HookDry<S[k]>;
	};

	type withStoreType<S extends initialType> = {
		[k in keyof S]: S[k] extends initialType ? withStoreType<S[k]> : HocFunc<S>;
	};

	type initialType = Record<string, any>;

	type afterCallbackType<S extends initialType> = (param: {
		store: S;
		prevStore: S;
	}) => void;

	function createStore<S extends initialType>(
		initial: S,
		afterCallback?: afterCallbackType<S>
	): {
		useStore: Hook<S> & useStoreType<S>;
		getStore: HookDry<S> & getStoreType<S>;
		withStore: HocFunc<S> & withStoreType<S>;
	};

	export default createStore;
}

useStore and getStore are smart typed with initial datas:

const initialStore = {
	counter: 0,
        label: 'Counter'
};

const { useStore, getStore, withStore } = createStore(initialStore);

function App() {
        // count: number ; setCount: (value: number) => void
	const [count, setCount] = useStore.counter(); // Same with getStore

        // store: { counter: number, label: string } ; setStore: (value: { counter: number, label: string }) => void
	const [store, setStore] = useStore(); // Same with getStore

	const handleCount = useCallback(() => {
		setCount(count + 1);
	}, [count, setCount]);

	return (
		<div>
			<h1>Counter:</h1>
			<p>{count}</p>
			<div>
				<button onClick={handleCount}>Incrémenter</button>
			</div>
		</div>
	);
}

Class based component is a little more complex:

import createStore, { Hoc } from 'teaful';

const initialStore = {
	counter: 0
};

const { withStore } = createStore(initialStore);

// A store path must be provided to the React.Component to inject this.props.store in component
// ex: Use withStore.counter(App) with Hoc<typeof initialStore.counter>
//      Use withStore(App) with Hoc<typeof initialStore>
//      Etc
class App extends React.Component<Hoc<typeof initialStore.counter>> {
	render() {
		const [count] = this.props.store;
		return <div>counter: {count}</div>;
	}
}

const AppWithStore = withStore.counter(App);

Waiting for the official feature, that's do the job

@Chetan-unbounce
Copy link

@laekem34 type def looks good, one small thing I noticed is
`
type HookReturn = [T, (value: T) => void, () => {}];

// should this be
type HookReturn = [T, (value: T) => void, () => void];

`

@laekem34
Copy link

laekem34 commented Nov 9, 2021

Yes, you're right 😄
Thanks for correction

@aralroca
Copy link
Collaborator

aralroca commented Nov 9, 2021

Surely both of you are more familiar with TypeScript than I am, so I trust the proposal 😊. Thank you for the proposal 👏👏

@Chetan-unbounce
Copy link

one more thing we should verify is, how would adding dynamic store keys work with the typing. And should we have Store properties being added dynamically, Store should be more static.

@laekem34
Copy link

laekem34 commented Nov 9, 2021

Surely my proposal isn't complete and could be improved.
I learnt this typescrit level recently. I write that tearful types because it is efficient and simple to use. Thanks for create it.
But don't hesitate to improve this type declaration

@danielart
Copy link
Collaborator

great work here, we could review it and set it on project. I'll try to spend time on this thread tomorrow if the proposal is still ongoing. Thanks both for your ideas!

@aralroca aralroca modified the milestones: 0.8.0, Before 1.0.0 Nov 14, 2021
@mspae
Copy link

mspae commented Nov 14, 2021

Thanks for the library & the typings. Just started using it on a project and I love it.

I did run into one issue though:

import React from "react";
import createStore from "teaful";

type IState = {
  foo: {
    bar: number;
  };
};
const defaultState: IState = {
  foo: {
    bar: 1,
  },
};
const { useStore, getStore } = createStore(defaultState);

export const FooComponent = () => {
  const [bar, setBar] = useStore.foo.bar(); // this works just fine
  const [foo] = useStore.foo();
  //                     ^^^
  // This expression is not callable.
  // Type 'useStoreType<{ bar: number; }>' has no call signatures.ts(2349)
  // (property) foo: useStoreType<{
  //     bar: number;
  // }>

  return null;
};

This change fixed it for me:

// Replacing this ...
type useStoreType<S extends initialType> = {
     [k in keyof S]: S[k] extends initialType ? useStoreType<S[k]> : Hook<S[k]>;
};
// With this ...
type useStoreType<S extends initialType> = {
    [k in keyof S]: useStoreType<S[k]> & Hook<S[k]>;
};

EDIT: This would probably also apply to getStoreType in the same way. BTW I'm not sure what the implications of using the intersection type in this way are – there might be something here I haven't quite unterstood.

EDIT 2:
So based on my limited unerstanding I think this might be the correct way: ( EDIT 3: Okay so this doesn't work after all, the above solution works)

type useStoreType<S extends initialType> = {
    [k in keyof S]: S[k] extends initialType
      ? useStoreType<S[k]> & HookDry<S[k]>
      : HookDry<S[k]>;
};

type getStoreType<S extends initialType> = {
    [k in keyof S]: S[k] extends initialType
      ? getStoreType<S[k]> & HookDry<S[k]>
      : HookDry<S[k]>;
};

@acroyear
Copy link

I've incorporated the above into an index.d.ts along with index.js into my own project and I'll keep you up to date w/ what i find.

@acroyear
Copy link

acroyear commented Nov 21, 2021

And I must be doing something wrong, because I'm still getting my useStore method returning "any", rather than the types for the expected state hook [v, setV] array.

also, when typescript compiles it, the index.d.ts file is lost and i'm back to the same code in the lib folder as the original 4-line excerpt above, everything an 'any'.

@Chetan-unbounce
Copy link

@acroyear so the code below loops through the state object, t give you hooks to set/read each attribute for the root store object, and it does by recursively flattening any object value. So that's why we don't get the hook for "foo".

I would advocate for keeping the root state flat, and splitting the state to smaller state object if there is nested objects. As the purpose of this lib is to fragment the state object into smaller(primitive) state read/write.

But I do see your point where someone could need a root state with nested state object and in that case we should flatten it for both the object and its properties.

    [k in keyof S]: S[k] extends initialType ? useStoreType<S[k]> : Hook<S[k]>;
  };

so this works - https://codesandbox.io/s/adoring-architecture-r4yvf?file=/src/App.tsx

type useStoreType<S extends initialType> = {
    [k in keyof S]: S[k] extends initialType
      ? useStoreType<S[k]> & Hook<S[k]>
      : Hook<S[k]>;
  };

but this could be something to discuss, as its more about

  • structuring the app and state and
  • what's the final motive of this library is, what level of fragmentation is aiming at.

@danielart
Copy link
Collaborator

danielart commented Nov 25, 2021

Hi folks! @Chetan-unbounce, @laekem34 , @aralroca

We were working and discussing over types, we created a MR ( #52, still on draft ). Honestly the work done here by you is awesome. We developed the types from zero and the result was same than yours, so we would like to give the credit from this to you and I propose to put you as contributors, you really deserve it.

Having this in mind, could be great if you have time to check the MR and feel free to ask for changes or improvements. We have to test it deeply before merge it, but we make a good step together with this, so big thanks to all of you working on this ❤️

@danielart
Copy link
Collaborator

created canary release to test this MR -> 0.9.0-canary.2

@Chetan-unbounce
Copy link

@danielart and @aralroca happy to help more, in development or testing side.

@aralroca
Copy link
Collaborator

aralroca commented Dec 2, 2021

@Chetan-unbounce thank you! Do you have some idea how to fix this?

image

Property 'postData' does not exist on type 'HookDry<{ postId: number; }> & getStoreType<{ postId: number; }>'.

When the property is new in the store (not defined inside the initialStore). I was trying to find a solution but I'm a bit lost. Thanks 👏

https://codesandbox.io/s/flamboyant-dust-tmer6?file=/src/App.tsx

@danielart
Copy link
Collaborator

danielart commented Dec 2, 2021

I was wondering if we can do this

type getStoreType<S extends initialStoreType> = {
  [key in keyof S | string]: S[key] extends initialStoreType
    ? useStoreType<S[key]> & HookDry<S[key]>
    : HookDry<S[key]>;
};

but having in mind that always that a property is called and doesn't exists on store we're creating it, may we could remove. the keyof S completely

@aralroca
Copy link
Collaborator

aralroca commented Dec 2, 2021

I found this solution before, but this is lost:

image

😅

@aralroca
Copy link
Collaborator

aralroca commented Dec 2, 2021

I released 0.9.0 with the types that @danielart added. Any improvements for what we have already discussed will be fixed by 0.9.x. Any PR will be very welcome!

@Chetan-unbounce
Copy link

@aralroca I might have a slight different opinion on adding the properties on the fly to the Store. Since we might be passing the interface for the Store when we call create store to get the typings working with generics. It does not make sense to add properties on the fly. A better approach would be to define them upfront and just grab the fragment when needed. This way a dev to easily know what priorities in the store exist and might also avoid potential bugs of creating properties on the fly when not required or overriding existing ones.

const { useStore, getStore } = createStore<StoreType>(defaultData);

@aralroca
Copy link
Collaborator

aralroca commented Dec 2, 2021

@Chetan-unbounce Thanks for your answer. For me it makes sense to have all the types of the store in the beginning, in fact, I find it very useful.:

image

However, it doesn't quite work. It's fixed providing postData as undefined inside the default values:

const { useStore, getStore } = createStore<StoreType>({ postId: 9001, postData: undefined });

That's the weird thing. I find this is redundant (it's already defined in the type), and besides that having to put all the properties as undefined increases the size of the bundle and the work of the developers.

@Chetan-unbounce
Copy link

@aralroca so i looked into the and issue is that we have the Store types as
type StoreType = { postId: number; postData?: PostData; };
where we say postData can be undefined or it can be PostData, and the error you see with typescript on this line

const [, setPostData] = getStore.postData();

(property) postData?: HookDry<PostData | undefined> | undefined Object is possibly 'undefined'.ts(2532) Cannot invoke an object which is possibly 'undefined'.ts(2722)

this is pretty valid as postData is set to be either of those values and when in the type definition we try to do S[key] it returns undefined, thats how typescript treats optional value.

and the code is working fine its just the typescript that cannot statically analyze the value so in that case we use the "!" operator

const [, setPostData] = getStore.postData!();

here is the forked link https://codesandbox.io/s/nervous-waterfall-kqp4f?file=/src/App.tsx:646-691

@aralroca
Copy link
Collaborator

aralroca commented Dec 2, 2021

@Chetan-unbounce Thank you for the clarification! 😊

So after all, no fix in types is needed in the library @danielart, what will be needed is to document well the TypeScript part for people who are not so familiar (like me 😅). In my case, I now intend to incorporate TypeScript into my day-to-day life. It's something I want to change today.

The next thing would be that everything in the Teaful repo is moved to TypeScript.

@aralroca aralroca added this to Progress in 1.0 Dec 3, 2021
@aralroca aralroca moved this from Progress to Done in 1.0 Dec 3, 2021
@aralroca
Copy link
Collaborator

aralroca commented Dec 3, 2021

I close the issue. It remains to move the entire project from Teaful to TypeScript but I prefer to create a new issue for this. Any issue on any type feel free to open an issue or directly PR it, it will be very welcome! Thanks! 🙏

@aralroca
Copy link
Collaborator

aralroca commented Dec 20, 2021

@laekem34 I have doubts about the benefits of the Hoc type:

import createStore, { Hoc } from 'teaful';

const initialStore = {
	counter: 0
};

const { withStore } = createStore(initialStore);

// A store path must be provided to the React.Component to inject this.props.store in component
// ex: Use withStore.counter(App) with Hoc<typeof initialStore.counter>
//      Use withStore(App) with Hoc<typeof initialStore>
//      Etc
class App extends React.Component<Hoc<typeof initialStore.counter>> {
	render() {
		const [count] = this.props.store;
		return <div>counter: {count}</div>;
	}
}

const AppWithStore = withStore.counter(App);

Does it have more benefits than using ReturnType with the useStore?

import createStore from 'teaful';

const initialStore = {
	counter: 0
};

type storeType = {
      counter: number,
      username?: string
}

const { useStore, withStore } = createStore<storeType>(initialStore);

type Props = {
      store: ReturnType<typeof useStore.counter>
}

class App extends React.Component<Props> {
	render() {
		const [count] = this.props.store;
		return <div>counter: {count}</div>;
	}
}

const AppWithStore = withStore.counter(App);

I think (IMHO) it is better to have all the store types to a single place and thus serves if the property is optional and is not within the initialStore (for example if you want to consume the username in a component)

However I would like to know your opinion, since you have more experience than me in TypeScript and I appreciate the great proposal you made. Thank you very much. ☺️

@aralroca aralroca reopened this Dec 20, 2021
1.0 automation moved this from Done to Progress Dec 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
1.0
Progress
Development

No branches or pull requests

7 participants