Tslint: Warn for import statements with side-effects

[mgechev]

Lets suppose we have the modules P, C1 and C2. P loads C1 and C2 lazily. Module C1 has the following imports:

import { Observable } from 'rxjs/Observable';
import 'rxjs/add/observable/of';

And C2 has:

import { Observable } from 'rxjs/Observable';
import 'rxjs/add/operator/mapTo';

Now lets suppose in tsconfig.json's files property we have listed the entry files for P, C1 and C2. Since the three modules won't be bundled together, we can get a runtime error in the following cases:

• User navigates to P, which loads C1 which uses mapTo (referenced in C2).
• User navigates to P, which loads C2 which uses Observable.of (referenced in C1).

Behind the hood, rxjs dynamically adds the mapTo method to the prototype of Observable, and of to the constructor function.

Although it is possible to apply type checking as a way for verification of correctness in these cases, it's hard to combine the proper set of files to be compiled together in a real-life project (i.e. in this case P together with C1 and P together with C2).

In this case, it might be a good idea to provide warning to the user in case of side-effect import statements (original source https://github.com/angular/angular-cli/issues/3904).

We can have a rule which if finds:

import 'path-to-module';

Produces a warning import with explicit side-effect, and has a description: Avoid import statements with side-effect.

Do you think such rule will be a good fit for the set of core rules of tslint? I'd love to help with a PR.

// cc @hansl @filipesilva

[mohsen1]

This rule should have an option to ignore modules that match a regular expression. CSS files are usually imported to have a side effect:

import "./style.css";

[mgechev]

@mohsen1, makes sense. This changes the rule to:

"no-import-side-effect": [true, { "ignore-pattern": "/rxjs.*/" }]

And schema:

{
  "type": "array",
  "items": {
    {
      "type": "object",
      "properties": {
        "ignore-pattern": {
          "type": "string"
        }
      },
      "additionalProperties": false
    }
  },
  "minLength": 0,
  "maxLength": 1
}

A PR is coming sometime next week.

[johnpapa]

I think this topic begs the question of where and when do we import these? We could import each operator and function for rxjs in every place it is used. Upside is you get exact behavior, downside is you have tons and tons of import statements.

Or we could import at the root of a module that uses the features. Upside is reduced imports but still have the behavior we want within a module.

Or we could import at a root module only. This is a bit more risky if a sub module could also be used elsewhere, as its imports wouldnt travel.

cc @wardbell

[hansl]

@johnpapa I think that discussion would be better suited in another channel. Having the lint check is useful and it's up to the project to enforce an appropriate style guide.

[johnpapa]

@hansl sure. I wasnt sure where that discussion should happen ... so lead the way and I will follow :)