fix(Notification): replace const enum

[cartant]

Description:

This PR replaces the NotiticationKind const enum that was introduced in 6.4.0 with a string-literal union type. The latter is type-safe and compatible with single-module transpilation. Const enums are not, as the transpiler cannot know what should be used as a replacement for a reference to a const enum without using type information.

The NotiticationKind const enum is a problem for any projects using the --isolatedModules flag or for projects using the Babel TypeScript transform - which means any TypeScript projects generated with create-react-app.

Related issue (if exists): #4538

[cartant]

BTW, I attempted to add a transpile script to reproduce the error - and guard against it in the future - but I was unable to get it to work, as TypeScript seems to only perform the check that effects that error for packages consumed from node_modules. Consuming them from dist effects a truckload of other errors about re-exports also not being compatible with --isolatedModules.

[coveralls]

Pull Request Test Coverage Report for Build 8266

• 7 of 7 (100.0%) changed or added relevant lines in 1 file are covered.
• 7 unchanged lines in 4 files lost coverage.
• Overall coverage decreased (-0.3%) to 96.687%

---

Files with Coverage Reduction	New Missed Lines	%
src/internal/observable/ConnectableObservable.ts	1	55.46%
src/internal/operators/partition.ts	1	75.0%
src/internal/Observable.ts	1	91.67%
src/internal/util/not.ts	4	20.0%

Totals
Change from base Build 8252:	-0.3%
Covered Lines:	5253
Relevant Lines:	5433

---

💛 - Coveralls

[mrmckeb]

This is great, and will allow us to move to v6.4. Are there any concerns about merging this in?

[benlesh]

Unfortunately, this is a breaking change. Anyone using the NotificationKind enum like NotificationKind.NEXT will be broken with this change.

[benlesh]

I think if we just leave the enum where it is, and convert everything else to just use 'N'|'E'|'C' we should be able to resolve the bug.

We can then @deprecate NotificationKind.

[cartant]

@benlesh Unfortunately, this has to be a breaking change. It's the export that is the problem - regardless of whether or not NotificationKind is used:

node_modules/rxjs/internal/Notification.d.ts:3:27 - error TS1209: Ambient const enums are not allowed when the '--isolatedModules' flag is provided.
3 export declare const enum NotificationKind {

The introduction of the const enum was a breaking change for many people - myself included - so I see no reason the fix cannot also be a breaking change.

[kwonoj]

Maybe just const enum to enum?

[cartant]

@benlesh I've updated the PR to use a literal union in the Notification signature - with NotificationKind being kept as a non-const enum. A const enum cannot be exported without effecting an error in situations in which isolated modules are used.

I've added a deprecation - with a message - to the enum. In the next major version, the enum could be ditched and NotificationKind could be declared as a type alias for the literal union.

[mrmckeb]

It looks like this is actually passing, the Travis error seems to be unrelated. Any chance we can get this in now?