Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

What's the recommended method for using rxjs-add in a modularized Angular app? #30

Closed
kamok opened this issue Jan 19, 2018 · 4 comments
Closed

Comments

@kamok
Copy link

kamok commented Jan 19, 2018

I've used this package along with this configuration here in my tslint.json:

"rxjs-add": {
      "options": [
        {
          "allowElsewhere": false,
          "allowUnused": false,
          "file": "./src/rxjs.imports.ts"
        }
      ],
      "severity": "error"
    }

It has worked perfectly so far, with a centralized rxjs.imports file to keep track of all the operators and observable methods.

However, since I've refactored into using an ngModule style of file structure (for lazy loading and just tidier file structures), the Unused patched observable in ./src/rxjs.imports.ts: fromEvent errors are firing off falsely.

Here's the article I used to modularize: https://medium.com/@motcowley/angular-folder-structure-d1809be95542.

The structure now looks like this for src/app

- app.module.ts
- app.component.ts
- modules
    - module1
       - components
       - pages
       module1.service.ts
       module1.module.ts
       module1.routes.ts

    - module2
       - components
       - pages
       module2.service.ts
       module2.module.ts
       module2.routes.ts
- shared
   - components
   - mocks
   - models
   - ...

What kind of configuration do I need to make in order to point them to my modules?

Mind you, the app works perfectly. It's just the allowUnused rules that are falsely firing.

@cartant
Copy link
Owner

cartant commented Jan 20, 2018

The options for the rule look fine. There is no real recommendation regarding those. However, if you are using the Angular CLI you should follow the configuration recommendations in the README or look at what was done in the included example.

My figuring out what's happening is close to impossible, but I do have some questions that you can ask yourself and investigate:

  • If you are using the Angular CLI, have you ejected? Have you changed the linting configuration at all?
  • The Angular linting runs in three passes. Which fails? Do any succeed?
  • Moving the files around should not be a problem. I have CLI projects with a structure similar to what you've outlined. Was anything else changed when it was restructured?
  • Are you sure all of the files are being linted? If a file is not linted, its usage of a patching observable or operator will not count and said patching could be reported as unused. To ensure the files you expect to be linted are so, why not add some code to intentionally fail the linting?
  • Is it only fromEvent? Have you changed the code to use fromEvent as an imported function, rather than a patching observable?

Hope that's helpful.

@kamok
Copy link
Author

kamok commented Feb 8, 2018

I think my issue is similar to #33

I saw what you wrote on that Issue and I've tried the following:

In my auth.service.ts, I have this line:

const foo = Observable.of([]);

This gets rid of the ERROR: ../client/src/rxjs.imports.ts[8, 1]: Unused patched observable in ./src/rxjs.imports.ts: of

Any ideas why the linter isn't picking up the usage in the actual class?

This isn't really a big deal, imo. Just a nice to fix, fix.

@cartant
Copy link
Owner

cartant commented Feb 8, 2018

The problem is that it's very difficult to see the whole context. I'm not even sure what the class you mention looks like or what the usage of Observable.of is like within it.

Basically, the three-pass linting that's in the CLI makes dealing with a single imports file a little tricky - doable, but somewhat tricky. However, it makes diagnosing problems almost impossible. I think that the only way these sorts of things could be addressable would be if the problem could be reproduced - in a fork - using the CLI example that's in this repo.

All I can suggest - without a repro - is that you do what's mentioned in the README:

  • switch of the unused rule for the e2e tests; and
  • make sure all of the RxJS imports in the single import file are used in both the tests and the app.

@cartant
Copy link
Owner

cartant commented Feb 20, 2018

Closing this, as without a repro, there's not much I can do.

However, I realise that I didn't ask whether or not you were using TSLint's no-unused-variable rule. I've since added an issue template with the following note:

Please ensure that TSLint's no-unused-variable rule is not enabled in your configuration (or in any configuration that you extend). That rule has caused problems in the past - as it leaves the TypeScript program in an unstable state - and has a significant number of still-open issues.

@cartant cartant closed this as completed Feb 20, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants