Can't subscribe to a record with record.subscribe(callback) with callback being the only param because of Typescript definitions. about deepstream.io-client-js HOT 7 CLOSED

Hi, I'm not using TS on my apps so I haven't encountered this issue, would setting path as optional work :
public subscribe (path?: string, callback: (data: any) => void, triggerNow?: boolean) { ?

Another option would be to set path type as string | null and then when subscribing to the complete record you pass null as path (that works currently)

from deepstream.io-client-js.

Hi,

I'd personally go with the second solution. However, that would be a breaking change.
Setting the path to optional won't work because a required parameter cannot follow an optional parameter.

So the question remains, is it ok to introduce a breaking change to the client? 😊
(I mean to introduce a breaking change to the non-TS usage of the client as well.
Currently, it is not consistent and I think it should be.)

If yes, I'll create a PR.

from deepstream.io-client-js.

Let me recap and see if we are talking about the same:

1. The TS compiler throws error when you do record.subscribe(callback) due to typing issues, it requires a defined path with a string value (I haven't veryfied this, but seems legit from the source)
2. In the current client you can subscribe to the complete record by doing record.subscribe(callback) or record.subscribe(null, callback) : both options work

Possible solutions:

1. Set the path type to string | null and thus TS users would have to call the function as record.subscribe(null, callback) for subscribing to the complete record and wouldn't have type problems from the TS compiler. As far as I see this won't be a breaking change for JS users, although a nasty difference between TS and JS usage.

2. Follow my linter suggestion :) Set the record.subscribe function parameters as unused by prefixing an underscore: _path, _callback, _triggerNow : could you check if this solves your issue?

from deepstream.io-client-js.

Yes, it seems that we are on the same page. Thanks for the clarification.

Prefixing the params doesn't solve the issue.

I'd go with setting the path type to string | null

I was talking about a breaking change because my initial thought was not to allow different usages for TS and JS users.
However, if you say that we can live with the non-consistent calls, then I would recommend updating the docs to promote the record.subscribe(null, callback) usage for everyone from now on.

from deepstream.io-client-js.

Another option, take a look at the same record.ts source here for example : multiple definitions are used in order to cover all possible cases. Maybe that's the rigth way to go. Hope you have a chance to test it, that would be the ideal scenario, no changes needed to documentation or anything else.

from deepstream.io-client-js.

Great suggestion. 👍 Works like a charm. PR sent.

from deepstream.io-client-js.

Excelent

from deepstream.io-client-js.