search

ReactiveX / rxjs-tslint

TSLint rules targeting RxJS
MIT License
309 stars 41 forks source link

PipeableOperatorsOnlyRule.js fails silently when RXJS 6 is installed #54

Closed bedag-moo closed 5 years ago

bedag-moo commented 6 years ago

As recommended by the angular update guide, I updated to rxjs 6 before running rxjs-5-to-6-migrate.

rxjs-5-to-6-migrate ran without errors, and correctly fixed the imports, but left patched operator calls unchanged.

Looking at the implementation of the PipeableOperatorsOnlyRule.js, I noticed that is uses the typeChecker to identity instance operator calls. Suspecting that the type checker needs the source code to compile, and therefore the old rxjs in node_modules, I ran 

npm install rxjs@^5.0.0
rxjs-5-to-6-migrate -p src/tsconfig.app.json

and the patched operator calls were rewritten correctly.

Please consider emitting an error message if the wrong rxjs version is installed - if I hadn't seen rxjs-5-to-6-migrate rewrite patched operator calls in another project, I would have assumed it incapable of doing so, and missed out on a great feature.

CanKattwinkel commented 6 years ago

Thansk @bedag-moo - saved my day.

ydomenjoud commented 5 years ago

should be on readme first page

dsteegen commented 5 years ago

I was also looking into this issue for more than an hour until I found this issue. Thanks for pointing that out!

ruimarques commented 5 years ago

I am so glad I perused rxjs-tslint issues when this tool failed to update the operators... Thank you @bedag-moo

mgechev commented 5 years ago

@bedag-moo thanks for pointing this out. I'll update the update guide and the readme of the project today.

mgechev commented 5 years ago

@bedag-moo just went through a sample migration. When you run ng update @angular/core, Angular CLI would install rxjs-compat. PipeableOperatorsOnlyRule will take the typings from there and the migration should go as expected.

I'll update the readme once again to reflect the findings.

bedag-moo commented 5 years ago

After 3 months, my memory is a bit hazy ... I guess it is possible that I ran npm install rxjs@^6.0.0 rather than ng update because I didn't realize that the phrase

Use ng update or your normal package manager tools to identify and update other dependencies.

was not meant to include rxjs.

In any case, rxjs-5-to-6-migrate should error if it can not resolve the Observable type rather than silently doing nothing.