Conversation
| cleanup() | ||
| // @ts-expect-error | ||
| resolve() // noop | ||
| } |
There was a problem hiding this comment.
Previously this let + late declaration was required because it'd complain that onDisconnected was being used before it was defined, probably supposed to detect temporal deadzone e.g. in connection.off('disconnected', onDisconnected). I guess something just got smarter about this rather than the use-before-define detection or whatever being disabled?
There was a problem hiding this comment.
To be honest I was a bit uneasy changing these, but assumed they should work. The previous version complained about using let:
/home/harbu/work/streamr-client-javascript/src/stream/utils.ts
113:13 error 'onDisconnected' is never reassigned. Use 'const' instead prefer-const
However it does feel a bit off referencing a symbol that only gets defined later on...
| resolve(value) | ||
| } | ||
| onError = (error) => { | ||
| const onError: (error: Error) => void = (error) => { |
There was a problem hiding this comment.
Well I checked and seems like TypeScript doesn't work any magic around this, the compiled output is
function waitFor(emitter, event) {
return new Promise((resolve, reject) => {
const onEvent = (value) => {
emitter.off('error', onError);
resolve(value);
};
const onError = (error) => {
emitter.off(event, onEvent);
reject(error);
};
emitter.once(event, onEvent);
emitter.once('error', onError);
});
}There was a problem hiding this comment.
Ah, we might wanna re-enable rule https://eslint.org/docs/rules/no-use-before-define @timoxley and revert these changes , agree?
| const onDisconnected = () => { | ||
| cleanup() | ||
| // @ts-expect-error | ||
| resolve() // noop |
There was a problem hiding this comment.
This // @ts-expect-error will probably go away with a resolve(undefined)
| import { StreamrClient } from '../src/StreamrClient' | ||
| import { PublishRequest } from 'streamr-client-protocol/dist/src/protocol/control_layer' | ||
|
|
||
| // eslint-disable-next-line @typescript-eslint/no-var-requires |
There was a problem hiding this comment.
These can actually probably be regular import calls.
a4a4dfb to
411d79d
Compare
Use shared eslint-config-stream-ts in line with streamr-dev/network#555. Had to add in a few additional rules to accommodate ongoing transition into TypeScript. Particularly rules:
@typescript-eslint/ban-ts-commentmany instances of@ts-expect-errorwithout description. Not worth adding in descriptions imo since they will be most likely replaced with types later on.no-underscore-dangleagain a lot of private fields leftover from javascript days, prolly worth changing when working on said code to prevent unexpected changes in behavior.requirestatements left. They were tactically placed, so I assumed they are most likely there on purpose. Annotated such with the following// eslint-disable-next-line import/no-unresolved.