style(Notification): Use enum to safely type the notification kinds

[zakhenry]

Description:
Rather than using magic strings which are easily use incorrectly, this PR introduces a enum NotificationKind.

This is not a breaking change as all clients that happen to already test the kind against a string will still have the same behaviour, both at compiletime and runtime, with the exception that if they use a notification kind letter that does not exist, they will get a compile time error.

[ci-reporter]

The build is failing

✨ Good work on this PR so far! ✨ Unfortunately, the Travis CI build is failing as of f7332f5. Here's the output:

npm test

> @reactivex/rxjs@6.3.3 test /home/travis/build/ReactiveX/rxjs
> cross-env TS_NODE_PROJECT=spec/tsconfig.json mocha --opts spec/support/default.opts "spec/**/*-spec.ts"


/home/travis/build/ReactiveX/rxjs/node_modules/ts-node/src/index.ts:250
    return new TSError(diagnosticText, diagnosticCodes)
           ^
TSError: ⨯ Unable to compile TypeScript:
spec/Notification-spec.ts(15,32): error TS2345: Argument of type '"x"' is not assignable to parameter of type 'NotificationKind'.

    at createTSError (/home/travis/build/ReactiveX/rxjs/node_modules/ts-node/src/index.ts:250:12)
    at getOutput (/home/travis/build/ReactiveX/rxjs/node_modules/ts-node/src/index.ts:358:40)
    at Object.compile (/home/travis/build/ReactiveX/rxjs/node_modules/ts-node/src/index.ts:545:11)
    at Module.m._compile (/home/travis/build/ReactiveX/rxjs/node_modules/ts-node/src/index.ts:430:43)
    at Module._extensions..js (module.js:664:10)
    at Object.require.extensions.(anonymous function) [as .ts] (/home/travis/build/ReactiveX/rxjs/node_modules/ts-node/src/index.ts:433:12)
    at Module.load (module.js:566:32)
    at tryModuleLoad (module.js:506:12)
    at Function.Module._load (module.js:498:3)
    at Module.require (module.js:597:17)
    at require (internal/module.js:11:18)
    at /home/travis/build/ReactiveX/rxjs/node_modules/mocha/lib/mocha.js:231:27
    at Array.forEach (<anonymous>)
    at Mocha.loadFiles (/home/travis/build/ReactiveX/rxjs/node_modules/mocha/lib/mocha.js:228:14)
    at Mocha.run (/home/travis/build/ReactiveX/rxjs/node_modules/mocha/lib/mocha.js:536:10)
    at Object.<anonymous> (/home/travis/build/ReactiveX/rxjs/node_modules/mocha/bin/_mocha:582:18)
    at Module._compile (module.js:653:30)
    at Object.Module._extensions..js (module.js:664:10)
    at Module.load (module.js:566:32)
    at tryModuleLoad (module.js:506:12)
    at Function.Module._load (module.js:498:3)
    at Function.Module.runMain (module.js:694:10)
    at startup (bootstrap_node.js:204:16)
    at bootstrap_node.js:625:3

I'm sure you can fix it! If you need help, don't hesitate to ask a maintainer of the project!

---

This comment was automagically generated by ci-reporter. If you see a problem, open an issue here.

[kwonoj]

This is technically breaking changes since string enum only works > 2.4 typescript compiler.

[zakhenry]

@kwonoj hmm true, is there a stated minimum supported version of typescript for rxjs?

[kwonoj]

we don't explicitly and I feel we may need, but that's also breaking changes 😅 for next major.

[cartant]

It would be a non-breaking change if NotificationKind were to be declared as:

export type NotificationKind = 'N' | 'E' | 'C';

And doing so would also make it type safe.

[kwonoj]

True, but I would prefer const enum if possible.

[coveralls]

Pull Request Test Coverage Report for Build 7761

• 10 of 10 (100.0%) changed or added relevant lines in 1 file are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage increased (+0.002%) to 96.808%

---

Totals
Change from base Build 7749:	0.002%
Covered Lines:	5246
Relevant Lines:	5419

---

💛 - Coveralls

[benlesh]

I think this change looks great. Just need to discuss this with the team, and it'll have to wait for the next minor release (likely soon), since it's a new feature.

[kwonoj]

I like this but proposing next major and we explicitly declared supported ts version from next major.

[cartant]

LGTM.

This is technically breaking changes since string enum only works > 2.4 typescript compiler.

My understanding is that the minimum supported version of TypeScript is 2.8. That's the minimum we test against in CI.