Feature request: Prevent tag name collisions

[mischkl]

In order to avoid name collisions for Redux action types it is currently necessary (or at least best practice) to repeat the domain in the type name, for instance:

export const AuthActions = unionize({
  AUTH_LOGIN: ofType<LoginData>(),
  AUTH_LOGIN_SUCCESS: ofType<AuthData>(),
  AUTH_LOGOUT: ofType<void>(),
  AUTH_ERROR: ofType<any>(),
}, {
  tag: 'type',
  value: 'payload',
});

One way to alleviate the issue would be if the type string could be prefixed, for instance:

export const AuthActions = unionize({
  LOGIN: ofType<LoginData>(),
  LOGIN_SUCCESS: ofType<AuthData>(),
  LOGOUT: ofType<void>(),
  ERROR: ofType<any>(),
}, {
  tag: 'type',
  value: 'payload',
  tagPrefix:  'AUTH_'
});

Consumers could then reference things by their short names, e.g. AuthActions.LOGIN while the actual string behind the scenes would be AUTH_LOGIN, thus avoiding collisions with other non-auth domains.

However, this still wouldn't guarantee that tag name collisions wouldn't occur. For that a registry of tags would be necessary as well as a way to configure whether the tag names should be considered unique. The registry could have a structure like

const TAG_REGISTRY: {[tag: string]: string[]} = {
  'type': [
    'AUTH_LOGIN',
    'AUTH_LOGIN_SUCCESS',
    'AUTH_LOGOUT',
    'AUTH_ERROR',
  ]
};

and the configuration whether a tag should be considered unique could be done by an option unique: true or similar:

{
  tag: 'type',
  value: 'payload',
  tagPrefix:  'AUTH_',
  unique: true
};

Then if a tag name for the same tag (e.g. type) were to be used twice, an error could be thrown, thus preventing accidental action type collisions.

[pelotom]

It's fairly specialized to the Redux use case, but seems like it could be useful. Are you interested in making a PR for it?

[twop]

I was trying to do a namespace property. As far as I know you cannot do that without sacrificing type transparency. Example:

const Action = unionize({ ADD_TODO: ofType<{ text: string }>() });
// type is fully transparent
type ActionType = typeof Action._Union;

//which means you can construct it yourself
const add: ActionType = { tag: 'ADD_TODO', text: 'try namespace' };

// Let's try to prefix it
const PrefixAction = unionize(
  { Add: ofType<{ text: string }>() },
  { prefix: 'TODO_' }
);

type PrefixActionType = typeof Action._Union;
// the best you can get is {tag: string, text: string}. Not {tag: 'TODO_Add', ...}

// which means from ts perspective this is totally valid instance of PrefixActionType
const prefixedAdd: PrefixActionType = { tag: 'Add', text: 'try namespace' };

So unless you want to make PrefixActionType opaque (which means that you cannot construct it yourself) prefix is not feasible at the moment.

I wrote more details on this in https://github.com/pelotom/unionize/issues/18. Maybe I'm missing something though :)

[pelotom]

That’s a good point, we can make all the generated creators, marchers etc. work and use the prefix behind the scenes, but the type of the variants themselves will either be wrong or loosely typed 😕

[mischkl]

Well if the prefix thing isn't feasible at least the collision-prevention registry should be. :) I could look into doing a PR as soon as I have a free moment.

[twop]

Honestly I think unionize might not the best fit for redux actions. Partially because of the namespace issue.

Here are the redux actions libs that can provide better ergonomics (sry for promoting something else): https://github.com/cartant/ts-action https://github.com/luncheon/redux-typed-action

Example for type uniqueness (that's what I use at work):

const typeCache: { [label: string]: boolean } = {};
export function type<T>(label: T | ''): T {
  if (typeCache[label as string]) {
    throw new Error(`Action type "${label}" is not unique"`);
  }

  typeCache[label as string] = true;

  return label as T;
}

[mischkl]

@twop interesting, I thought Redux was one of the main reasons to use unionize. That's what it advertises in the README and for instance if you take a look at https://github.com/darrenmothersele/angular5-resolve-data you can see how the various features combined are quite convenient for that purpose. I took a look at your suggestions but IMHO the ergonomics are not significantly superior. redux-typed-action shortens the action creators but as far as I can tell that's it. ts-action does quite a bit more, but its API requires using lots of individually imported functions, which seems less obvious than the unionize way of doing things.

I am aware of things like the type() function you mention; the Ngrx example app used to include it and that was what got me thinking about how to do the same thing using unionize. I'm certainly open to the possibility of not including the functionality directly in the library but rather as an external function, if this functionality seems too Redux-y to include in unionize proper. However I guess it would basically mean parsing the inputs to unionize in the same way unionize parses them, prior to passing them to unionize, which means it is probably simpler to implement as part of unionize proper.

Alternately if you just assume that only one kind of tag is used throughout the application, the following use of the type() function seems to work, although it's not that convenient to type: 😉

export const AuthActions = unionize({
  [type('AUTH_LOGIN')]: ofType<LoginData>(),
  [type('AUTH_LOGIN_SUCCESS')]: ofType<AuthData>(),
  [type('AUTH_LOGOUT')]: ofType<void>(),
  [type('AUTH_ERROR')]: ofType<any>(),
}, {
  tag: 'type',
  value: 'payload',
});

I guess the deciding question is, what percentage of unionize users are using it with Redux? And/or are there other use cases where avoiding tag name collision would be helpful?

[twop]

I think you can write a helper function that would do the job:

// you would have to add some constraints on T to match unionize signature
function reduxActions<T>(actions:T) {
    Object.keys('actions).forEach( k=> ensureItIsUnique(k) )
    return unionize(actions, {tag:'type', value:'payload'})
}

And just use it instead of unionize. Or I'm missing something?

[pelotom]

You can verify this statically at the top level by combining the _Records of your various unions using a function which forbids reusing keys, something like

type ExcludeKeys<A> = Partial<Record<keyof A, never>> & Record<string, any>;

function combineUnique(): {};
function combineUnique<
  A
>(a: A): A;
function combineUnique<
  A,
  B extends ExcludeKeys<A>
>(a: A, b: B): A & B;
function combineUnique<
  A,
  B extends ExcludeKeys<A>,
  C extends ExcludeKeys<A & B>
>(a: A, b: B, c: C): A & B & C;
// ... as many overloads as needed
function combineUnique(...args: any[]) {
  return Object.assign({}, ...args);
}

const combined = combineUnique(
  { x: ofType<number>() },
  { y: ofType<boolean>() },
  { x: ofType<string>() }, // type error, tried to reuse `x`
);

[pelotom]

So, I guess now that I think about it, this might be a pretty useful combinator to have on the unionized object itself.

const Union1 = unionize(/*...*/);
const Union2 = unionize(/*...*/);
const Union3 = unionize(/*...*/);

// verifies statically that no tags are repeated
const Union4 = Union1.add(Union2).add(Union3);

[insidewhy]

I want something like:

const friendActions = unionize({
  cuddle: withTag('[Friend] cuddle').ofType({}),
})

const enemyActions = unionize({
  cuddle: withTag('[Enemy] cuddle').ofType({}),
})

Then I can have nice unique tags to read in my redux devtools.

[vladimiry]

I just hacked tagPrefix into the option object https://github.com/vladimiry/unionize/commit/fcf9b2d0e6bdeddcad45367d8772a185d65124b7. Also enabled lib dir pushing, but it's just to play around this change for a while to make sure it works as expected. Can extend tests and PR if that is not too ugly change.

@mischkl

unique: true

The thing is that a real-world app might have actions defined using different ways, not only the unionize one. You can't control how action defining/dispatching is happening in all the cases, means side effects in runtime would still be possible even having unique: true implemented here. If there is a need to verify uniqueness for actions constructed by unionize only, then I believe it can be done on top of it in runtime, like simply calling it through your wrapper that does caching and uniqueness validation, and then banning direct unionize import by linting rules. In my opinion enabling prefix feature would be a sufficient resolution.

[mlegenhausen]

Has anyone thought about wrapping your actions in a namespace type? That would increase reusability even more.

Example:

// app/counter/actions.ts
export const Actions = unionize({
  Decrement: ofType<{}>(),
  Increment: ofType<{}>()
}, { tag: 'type', value: 'payload' })
export const Actions = UnionOf<typeof Actions>;

// app/actions.ts
import * as CounterActions from '...';

export const Namespace = unionize({
  Counter: ofType<CounterActions>()
}, { tag: 'namespace', value: 'action' });

// Reducer
const reducer = (state, ns) => {
  return Namespace.match(ns, {
    Counter1: (action) => ({ ...state, counter1: Counter.reducer(state.counter1, action) }),
    Counter2: (action) => ({ ...state, counter2: Counter.reduce(state.counter2, action) })
  });
};

// Creating actions
const increment1 = Namespace.Counter1(CounterActions.Increment());
const increment2 = Namespace.Counter2(CounterActions.Increment());