5
\$\begingroup\$

So I'm learning typescript and I'm making a simple functional PubSub system. I want the user to be able to define topics and the data for those topics. So far I've got the below code, but I really want to improve it.

I'm not sure if I can get rid of the unknown in <T extends Record<string,unknown>>. I don't think that's a huge issue for me. I'd really like to get rid of the Set<Subscriber> part of the map that holds the subscribers. I tried something simple like Set<Subscriber<T[keyof T]>> but then it complains below where I am using <R extends keyof T>. Maybe that's the part that needs fixing?

Any and all suggestions welcome of course!

type Subscriber<R> = (data: R) => void;

const pubSubber = <T extends Record<string, unknown>>() => {
  const subscribers = new Map<keyof T, Set<Subscriber<any>>>();
  return {
    subscribe: <R extends keyof T>(topic: R, subscriber: Subscriber<T[R]>) => {
      const current = (subscribers.get(topic) || new Set()) as Set<
        Subscriber<T[R]>
      >;
      current.add(subscriber);
      subscribers.set(topic, current);
      return () => current.delete(subscriber);
    },
    publish: <R extends keyof T>(topic: R, data: T[R]) => {
      const toCall = Array.from((subscribers.get(topic) || [])) as Subscriber<T[R]>[];
      toCall.map((f) => f(data));
    },
  };
};

type Topics = {
  gear_with_radius: {
    center: number;
    radius: number;
  };
  outside: {
    help: boolean;
  };
};
const myPubSubber = pubSubber<Topics>();
myPubSubber.publish('gear_with_radius', { center: 10, radius: 5 });
\$\endgroup\$
2
  • \$\begingroup\$ There is something strange there. You are using subscribers.get(topic) as a list, but according to the type it should be set. \$\endgroup\$
    – KiraLT
    Nov 19, 2020 at 20:15
  • \$\begingroup\$ @KiraLT - yes, you are correct, my mistake. I'm missing an Array.from, will edit. \$\endgroup\$
    – Adam
    Nov 19, 2020 at 20:19

1 Answer 1

4
\$\begingroup\$

I have a few pointers regarding your snippet.

Return type

Let's start with pubSubber function return type. It's very nice that typescript infers types, but it also a good way to make a mistake. The bad thing about it is that you can accidentally change the return type without knowing it. If you use this function somewhere in the app there is a chance that typescript will notify you about the change. But if you are creating a library and don't have tests, you will change the return type without any error.

Personally, I like the rule to always specify return type (unless using inline arrow functions). I even use the lint rule to enforce this.

Don't use any

Change any to unknown. In this case, maybe it doesn't matter at least at the moment, but in the future, you may change this code. With any you can do anything and typescript won't say anything. With unknown you can't do anything, unless you cast it or use type Type Guard

const subscribers = new Map<keyof T, Set<Subscriber<any>>>()

Don't use cast if possible

Let's look into this line.

const current = (subscribers.get(topic) || new Set()) as Set<
  Subscriber<T[R]>
>;

current real type is Set<Subscriber<any>>, but you cast it to Subscriber<T[R]>. But in this example, there is no point having cast at all, because even without it the code is correctly typed. This function implementation works with any type, so you can leave it as it is. As for the outer interface, it can be more strict.

const current = subscribers.get(topic) || new Set()
current.add(subscriber);
subscribers.set(topic, current);
return () => current.delete(subscriber);

Second example is a bit better:

const toCall = Array.from((subscribers.get(topic) || [])) as Subscriber<T[R]>[];
toCall.map((f) => f(data))

You already had one mistake because of this cast. Also, I don't see any reason to convert Set to the list. You can just loop the Set. And if you combine it with optional chaining, it will look like:

subscribers.get(topic)?.forEach(f => f(data));

Better typing

In the previous section, I mentioned casts. But removing casts in theory you lose some typings (adding also lose some typings, dilemma...). Let's try to have full correct typings without any cast. So to start with we need to change Map to Object. Map types do not allow relations between key and value (e. g.: a key must-have number value`).

Example of relation-based subscribers list:

const subscribers: {
  [P in keyof T]?: Set<Subscriber<T[P]>>
} = {}

Final solution

So the final solution can looks like:

type Subscriber<R> = (data: R) => void;
type BaseTopics = Record<string, unknown>

interface PubSubber<T extends BaseTopics> {
  subscribe: <R extends keyof T>(topic: R, subscriber: Subscriber<T[R]>) => () => void
  publish: <R extends keyof T>(topic: R, data: T[R]) => void
}

function pubSubber<T extends BaseTopics>(): PubSubber<T> {
  const subscribers: {
    [P in keyof T]?: Set<Subscriber<T[P]>>
  } = {}
  return {
    subscribe(topic, subscriber) {
      const current = subscribers[topic] || new Set();
      current.add(subscriber);
      subscribers[topic] = current;
      return () => current.delete(subscriber);
    },
    publish(topic, data) {
      subscribers[topic]?.forEach(f => f(data));
    },
  };
};
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.