ReactiveX / rxjs

A reactive programming library for JavaScript
https://rxjs.dev
Apache License 2.0
30.78k stars 3k forks source link

RxJS should not rely on rxjs-compat #3985

Closed magleahy closed 6 years ago

magleahy commented 6 years ago

Bug Report

Current Behavior Have updated my app from Angular 5 to 6. By following the steps on https://update.angular.io/ - I have dealt with RxJs code changes e.g. imports, pipes etc. following the rxjs-tslint instructions.

After the Update

Remove deprecated RxJS 6 features using rxjs-tslint auto update rules.
For most applications this will mean running the following two commands:

`yarn global add rxjs-tslint`
`rxjs-5-to-6-migrate -p src/tsconfig.app.json`

Once you and all of your dependencies have updated to RxJS 6, remove rxjs-compat.

I remove rxjs-compat as instructed but RxJS relies on it. It should not?

Another issue is my tests do not run due to the rxjs-compat issue, it seems:

ERROR in node_modules/rxjs/Observable.d.ts(1,15): error TS2307: Cannot find module 'rxjs-compat/Observable'.

03 08 2018 14:41:28.976:WARN [karma]: No captured browser, open http://localhost:9876/
03 08 2018 14:41:28.983:INFO [karma]: Karma v2.0.5 server started at http://0.0.0.0:9876/
03 08 2018 14:41:28.983:INFO [launcher]: Launching browser Chrome with unlimited concurrency
03 08 2018 14:41:29.015:INFO [launcher]: Starting browser Chrome
03 08 2018 14:41:30.033:INFO [Chrome 67.0.3396 (Mac OS X 10.13.4)]: Connected on socket s44qFd54EqfF9utgAAAA with id 77754672
Chrome 67.0.3396 (Mac OS X 10.13.4): Executed 0 of 0 ERROR (0.001 secs / 0 secs)

Expected behavior Do not have RxJs rely on rxjs-compat or have a version that does not for those who are fixing their code early.

Environment

package.json

{
  "name": "project-name",
  "version": "0.0.0",
  "license": "MIT",
  "scripts": {
    "build-dev": "npm run config && ng build --configuration=dev",
    "build-preprod": "npm run config && ng build --configuration=preprod",
    "build-int": "npm run config && ng build --configuration=int",
    "build-uat": "npm run config && ng build --configuration=uat",
    "build-demo": "npm run config && ng build --configuration=demo",
    "build-prod": "npm run config && ng build --configuration=prod",
    "config": "ts-node ./scripts/set-env.ts",
    "e2e": "ng e2e",
    "lint": "ng lint",
    "ng": "ng",
    "serve-dev": "npm run config && ng serve --configuration=dev",
    "start": "ng serve",
    "test-dev": "ng test --code-coverage=true --source-map=true --browsers=Chrome",
    "test-ci": "ng test --watch=false --code-coverage=true --source-map=true --single-run=true --browsers=ChromiumHeadlessNoSandbox"
  },
  "private": true,
  "dependencies": {
    "@angular/animations": "^6.1.0",
    "@angular/cdk": "^6.4.2",
    "@angular/common": "^6.1.0",
    "@angular/compiler": "^6.1.0",
    "@angular/core": "^6.1.0",
    "@angular/forms": "^6.1.0",
    "@angular/http": "^6.1.0",
    "@angular/material": "^6.4.2",
    "@angular/platform-browser": "^6.1.0",
    "@angular/platform-browser-dynamic": "^6.1.0",
    "@angular/router": "^6.1.0",
    "@ngrx/effects": "^6.1.0",
    "@ngrx/router-store": "^6.1.0",
    "@ngrx/store": "^6.1.0",
    "@ngrx/store-devtools": "^6.1.0",
    "adal-angular4": "^3.0.5",
    "angular-highcharts": "^6.2.6",
    "angular2-uuid": "^1.1.1",
    "classlist.js": "^1.1.20150312",
    "core-js": "^2.5.7",
    "d3": "^5.4.0",
    "dotenv": "^6.0.0",
    "hammerjs": "^2.0.8",
    "highcharts": "^6.1.1",
    "jStat": "^1.7.1",
    "ng2-nouislider": "^1.7.11",
    "nouislider": "^11.1.0",
    "rxjs": "^6.2.2",
    "rxjs-tslint": "^0.1.5",
    "web-animations-js": "^2.3.1",
    "zone.js": "^0.8.26"
  },
  "devDependencies": {
    "@angular-devkit/build-angular": "~0.7.0",
    "@angular/cli": "^6.1.2",
    "@angular/compiler-cli": "^6.1.0",
    "@angular/language-service": "^6.1.0",
    "@types/d3": "^5.0.0",
    "@types/highcharts": "^5.0.24",
    "@types/jasmine": "^2.8.7",
    "@types/jasminewd2": "^2.0.3",
    "@types/node": "^10.5.5",
    "codelyzer": "^4.3.0",
    "jasmine-core": "^3.1.0",
    "jasmine-spec-reporter": "^4.2.1",
    "karma": "^2.0.2",
    "karma-chrome-launcher": "^2.2.0",
    "karma-coverage-istanbul-reporter": "^2.0.1",
    "karma-jasmine": "^1.1.2",
    "karma-jasmine-html-reporter": "^1.2.0",
    "ngrx-store-freeze": "^0.2.4",
    "protractor": "^5.3.2",
    "puppeteer": "^1.5.0",
    "ts-node": "^7.0.0",
    "tslint": "^5.9.1",
    "typescript": ">=2.7.2 <2.10"
  }
}

Possible Solution Remove the dependency on rxjs-compat.

Versions

Angular CLI: 6.1.2
Node: 10.8.0
OS: darwin x64
Angular: 6.1.0
... animations, common, compiler, compiler-cli, core, forms
... http, language-service, platform-browser
... platform-browser-dynamic, router

Package                           Version
-----------------------------------------------------------
@angular-devkit/architect         0.7.2
@angular-devkit/build-angular     0.7.2
@angular-devkit/build-optimizer   0.7.2
@angular-devkit/build-webpack     0.7.2
@angular-devkit/core              0.7.2
@angular-devkit/schematics        0.7.2
@angular/cdk                      6.4.2
@angular/cli                      6.1.2
@angular/material                 6.4.2
@ngtools/webpack                  6.1.2
@schematics/angular               0.7.2
@schematics/update                0.7.2
rxjs                              6.2.2
typescript                        2.9.2
webpack                           4.9.2
benlesh commented 6 years ago

Apparently, the tslint migration you ran didn't catch everything. If you're seeing that error, it's because you're importing from rxjs/Observable somewhere. That will cause rxjs to try to use rxjs-compat.

If you're getting notifications that you need rxjs-compat, it's because you're importing from somewhere that's not rxjs, rxjs/operators, rxjs/webSocket, rxjs/ajax or rxjs/testing.

I'm going to close this issue for now, but if you need further assistance let us know!

magleahy commented 6 years ago

Hi @benlesh - as mentioned above, I am running the rxjs-tslint on my code and all is well. Also I have checked all my rxjs imports and they all look right from what I know at this point. So I would like to respectfully challenge this issue being closed.