Rework subscribe function to add more typesafety

[kalvinpearce]

Need to look into if theres a better way of subscribing to state pieces. At the moment there is little safety around what gets put in the stateValue param. Would be good to rework to make it more typesafe if possible?

This would remove the need for branching statement and broken subscribes.

seorsum/src/index.ts

Lines 44 to 60 in 446d28f

	const subscribe = <T>(
	stateValue: (state: State) => T,
	callback: (state: T) => void,
	) => {
	// Little messy way of grabbing the state path from the params
	const res = stateValue.toString().match(/\.(.*);/);
	// This should always pass but is needed for intellisense
	if (res && res.length > 1) {
	const key = res[1];
	const fn = () => callback(stateValue(state));
	// Add to event bus
	bus.addListener(key, fn);
	// Return function to remove from event bus
	return () => {
	bus.removeListener(key, fn);
	};
	}

[kalvinpearce]

Something like this could be on the right track?
https://github.com/RafaelSalguero/DeepPath/

The idea is that you can curry the path out from an object type using keyof typed options.
Perhaps subscribe could be more along the lines of subscibe('name')('first');?

[kalvinpearce]

So this is now an important fix. The subscribing is based on the way you write the state piece out and if it compiles differently then it can break.
Since it is based on this regex it is quite easy to break and it breaks silently.

seorsum/src/index.ts

Line 49 in a90cefd

const res = stateValue.toString().match(/\.([a-z\.]*)/);

[kalvinpearce]

This looks like a promising solution rather than currying.
microsoft/TypeScript#12290 (comment)
With some adapting it means you could run subscribe(['name', 'first'1]); and each element in the array would be a string literal for the possible keys of state for that path. Limitation is that it only supports up to 7 layers deep but could be a good solution for now.

[kalvinpearce]

This is somewhat solved in #2 but I still feel like this needs to be left open and continue to investigate the solution. As @JamesYFC has suggested, maybe the solution could be to add more layers to the tuple path type but it seems limiting to have a hard upper limit of how deep the state can be until intellesense breaks.

[kalvinpearce]

@JamesYFC there is an issue with the new tuple path method that we missed. It seems to only support a depth of 2 at the moment

subscribe(['one', 'two']); // Works fine
subscribe(['one', 'two', 'three']); //Types of property '['2']' are incompatible.
      // Type 'string' is not assignable to type 'undefined'.

[JamesYFC]

Will take a look in a day or two, internet access is sketchy atm Sent from Yahoo Mail for iPhone On Tuesday, November 19, 2019, 1:19 pm, Kalvin Pearce <notifications@github.com> wrote: @JamesYFC there is an issue with the new tuple path method that we missed. It seems to only support a depth of 2 at the moment subscribe(['one', 'two']); // Works fine subscribe(['one', 'two', 'three']); //Types of property '['2']' are incompatible. // Type 'string' is not assignable to type 'undefined'. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

[kalvinpearce]

Closing now due to being fixed in b19bf2f

[kalvinpearce]

Current limit is 10 paths deep but after adding more I realised TS currently doesn't types of more than 10 deep... at 13 it will throw errors.