The сlientId type can be a number.

[mrdimidium]

The clientId type == string | false, but users can use number. You use number in @logux/redux tests, for example.

This is possible because when parameters are loaded, the userId is attached to the string.

To legalize using a number as a userId, I suggest adding it to the number type.

[ai]

Will it be better to be more strict here? I afraid of number ←→ string transformations with problems, which will comming.

client.userId = 10
parseId(meta.id).userId === client.userId //=> false, since "10" !== 10

[mrdimidium]

Perhaps you are right.

Note that we can allow you to specify the clientId type explicitly if we make the clientId generic.

Like this:

export type ClientOptions<U extends { toString(...args: any[]): string; } = string | false> = {
  clientId: U
};

Full example in CodeSandbox.

[ai]

Why do we need generic and not simple string | { toString(...args: any[]): string }?

[mrdimidium]

Because the generic value will be determined when the store is created. The simple type allows you to change the Id type after.

For example (in CodeSandbox):

interface SimpleClientOptions {
  clientId: { toString(...args: any[]): string } | false
};

interface GenericClientOptions<U extends { toString(...args: any[]): string } = string | false> {
  clientId: U
};

// For simple type
const sConfig: SimpleClientOptions = {
  clientId: "1"
}

sConfig.clientId = 1; // It's correct!

// For generic
const gConfig: GenericClientOptions = {
  clientId: "1"
}

gConfig.clientId = 1; // It's fail! clientId can be string only

const gConfig2: GenericClientOptions<number> = {
  clientId: 1
}

gConfig2.clientId = "2"; // It's fail! clientId can be number only

[ai]

Because the generic value will be determined when the store is created

Why do you need it?

The simple type allows you to change the Id type after.

BTW, Logux Client will not work if you changed client.options.userId after creating options.

[mrdimidium]

You know, I looked at it with fresh eyes and realized that you are right. There is no reason to use something in clientId instead of string.

However, Logux Client will now implicitly lead the clientId to string, and this may cause problems, because the application will continue to store the id as a number.

What do you think about refusing to cast the type, and additionally checking that typeof clientId === "string"?

It seems to be more obvious and reliable.

[ai]

However, Logux Client will now implicitly lead the clientId to string

Nope, we call toString

client/client/index.js

Line 56 in 7e33ffe

this.options.userId = this.options.userId.toString()

[mrdimidium]

Yeah, that's what I meant. Here's what the problem might be:

let userId = 10; // It's number

let creator = createLoguxCreator({
  userId // set number, like in tests
})
let store = creator(reducer, { value: 0 }, enhancer)

store.log.on('add', (action, meta) => {
  if (meta.clientId === clientId) { // meta.clientId => string, but clientId => number
    // No, because "10" !== 10
  }
})
store.dispatch.sync({ type: 'INC' })

I mean, the clientId in logux will be a string, whereas in an application it can be a number, or anything at all.

[ai]

Do you want to throw an error on non-string userId instead of calling toString()?

I like the idea that if you need type checks, you should use TS instead of JS.

[mrdimidium]

Yes, to throw errors seems like a good solution to me. This is more predictable behavior.

I am concerned that now typing describes not what can actually come, but what should be sent — there may be different behavior in JS and TS.

Do you think we should do anything about it?

[ai]

I will keep the current rule of using TypeScript for type safety for non-popular cases.