Subject.prototype.next() should only accept T, not T | undefined

[felixfbecker]

RxJS version: 5.4.3

Code to reproduce:

const subject = new Subject<string>()

subject.next()

Expected behavior:

Compile error

Actual behavior:

Compiles

Additional information:

next()'s argument is typed as optional, which means it allows undefined even if the Subject's type does not. It should only allow T, not T | undefined.

[kwonoj]

This.. seems makes sense to me? I think it's just kind of legacy when we deal with old version of TSC have limitation with union types.

[kwonoj]

/cc @david-driscoll

[felixfbecker]

Could we get this in before 6.0 as it's a slight breaking change?

[kwonoj]

It's breaking anyway, isn't it?

[felixfbecker]

What's breaking anyway?

[kwonoj]

Someone accidentally supplied undefined by not noticing it until. We had similar cases before, as long as it possibly breaking user code then we simply consider it as breaking even though it's obviously our side issue.

Honestly I'm not very tempted to touch type definition in v5 currently, lot of them are written with old versions of TSC compiler and we need revise huge breaking changes with v6 anyway, also changing v5 signature with old version surprisingly can affect user code in latest TSC. (not saying this one's those case)

[felixfbecker]

Yeah, that's why I'm asking whether we could get this change to be in 6.0

[kwonoj]

Oh, I thought you're asking it to be in 5.x. my bad. This should be in 6 in any cases.

[kwonoj]

Just clarifying lot of v6 issues are backlogged, cause next branch is not usable state so I'm not even accepting PR at this moment. We should release 5.5 and realign 6 branch asap.

[benlesh]

So, to be clear to others, if someone wants to use it as: subject.next() they would need to create a new Subject<void>().

[felixfbecker]

@benlesh not entirely sure whether that would work and might depend on your compilerOptions. At least in #3074 I had to explicitly call subject.next(undefined).