Should be possible to force import of "unused" modules in case there are side-effects

[markrendle]

TypeScript Version:

1.8.10

Code

MyService.ts

// A self-contained demonstration of the problem follows...
import {ngModule} from './../module.ts';

export class MyService {
}

ngModule.service('myService', MyService); // <- REALLY IMPORTANT SIDE-EFFECT

MyComponent.ts

import {MyService} from './MyService.ts';

class MyComponent {
  constructor(private myService: MyService) { }
  // ...
}

Expected behavior:
(I know this sort-of-tree-shaking is "by design", but by the principle of least astonishment, expected behavior is this)

MyComponent.js

var MyService_1 = require('./MyService');
var MyComponent = (function() {
  function MyComponent(myService) {
    this.myService = myService;
  };
  // ...
})();

Actual behavior:
MyComponent.js

var MyComponent = (function() {
  function MyComponent(myService) {
    this.myService = myService;
  };
  // ...
})();

And then AngularJS is all "myServiceProvider not found" and crashy.

[RyanCavanaugh]

It's possible:

import './MyService`;

[markrendle]

I know I can do that, but I shouldn't have to say

import './MyService';
import {MyService} from './MyService';

all over the place. It's just untidy.

[markrendle]

:)

[zpdDG4gta8XKpMCd]

I am too dumb for getting why import {MyService} from './MyService'; would not work, please enlighten.

[mhegazy]

please see https://github.com/Microsoft/TypeScript/wiki/FAQ#why-are-imports-being-elided-in-my-emit

[markrendle]

@Aleksey-Bykov Basically, in Angular code, the class is instantiated by the dependency injection system, so you import MyService but then you don't new it or anything, you just use it as the type for the injected parameter. TypeScript sees that you don't actually use the class except as a type declaration, which has no meaning in the emitted JavaScript, so it sees no reason to include it in the emitted JavaScript. It makes perfect sense, except when the file you are importing from has side-effects; in this case, the registering of the service with an AngularJS module.

[kitsonk]

While I get understand the use case, I can't conceive of a clean syntax that would denote that the import should not be elided... Only except maybe:

import *, { MyService } from './MyService';

In that the compiler would understand that the anonymous * is denoting that the import is being doing for side-effects but not being named. Therefore:

import * from './MyService';
/* is equivalent to */
import './MyService';

The one "edge case" is that if the default export of the module being something that gets erased (is that even possible to export default something that is erasable)?

[aluanhaddad]

@markrendle for what it's worth, this is my pattern for registering services in angular 1.x:

my-service.ts

export class MyService { 
    static $inject = ['$q'];
    constructor(readonly $q: ng.IQService) {}
}

my-other-service.ts

import { MyService } from './my-service';

export class MyOtherService { 
    static $inject = ['MyService'];
    constructor(readonly myService: MyService) {}
}

app.ts

import { module } from 'angular';
import { MyService } from './my-service';
import { MyOtherService } from './my-other-service';

const app = module('app', [])
    .service({
        MyService,
        MyOtherService
    });

export default app;

main.ts

import { bootstrap } from 'angular';
import app from './app';

bootstrap(document.body, [app.name], { strictDi: true });

This ensures the service import will not be elided. It also has the advantage of decoupling your service classes from angular (sure $inject is there but they don't depend on angular).

Edit: Updated example with explicit export from "app.ts"

[markrendle]

@aluanhaddad I see what you've done there, but I use ngAnnotate as part of my build process so I don't have to manually maintain $inject properties, and I don't think ngAnnotate would pick up that way of adding services... although I'll give it a try.

This would all go away if my boss would just let me spend 6-9 months rewriting the entire application in Angular 2...

[aluanhaddad]

@markrendle I don't use ngAnnotate so I wouldn't know if that would work but the only reason I write my "services" as classes registered with .service instead of factories registered with .factory is to take advantage of the static inject notation. That said, I only used the above style for my example because it is my preference.

My example was just meant to show that you don't have to register each service in the module where it is defined. I also think that there are compelling reasons to avoid doing so.

Regardless, using a tool like ngAnnotate shouldn't dictate the way in which you structure your code or the order in which you register your services.

[markrendle]

Regardless, using a tool like ngAnnotate shouldn't dictate the way in which you structure your code or the order in which you register your services.

@aluanhaddad But it does, because it makes life so much easier that I'm willing to put up with its demands.

[aluanhaddad]

@markrendle But if the import were not elided and the type were imported solely for use in type position in multiple dependent modules, you would end up registering the service multiple times. That just seems unsound.

Anyway, how about exporting a function which registers the service.

export class MyService { 
    static $inject = ['$q'];
    constructor(readonly $q: ng.IQService) {}
}

export default function register(ngModule: angular.IModule) {
    ngModule.service('myService', MyService);
}

[Cliveburr]

I'm going through this problem these days, exactly like @markrendle said.
What do you think of adding a flag to force the import, I had thought of something subtle, like the simbol ! already used to define important things in several places .. something like that..

import {MyService} from '!./MyService.ts';

[RyanCavanaugh]

! is already treated specially as part of AMD plugin paths

[Cliveburr]

@RyanCavanaugh Good thinking!

[aluanhaddad]

@Cliveburr @RyanCavanaugh I don't think this use case is valid. In @markrendle's example, if the module import were not elided when the imported entity is used only in type position, the side effect of registering the service would occur an additional time for each such import. It is common for multiple components to depend on the same service.

[RyanCavanaugh]

Are people seriously writing modules with non-idempotent load-time side effects? That seems insane

[aluanhaddad]

Yes they are, and yes it is insane. It works for them precisely because TypeScript elides these imports.
Here is a simple reproduction of the issue:

file0.ts

angular.module('app', [])
  .directive('myDirective', function() {
    return {
      scope: {}
    };
  });

file1.ts

angular.module('app')
  .directive('myDirective', function() {
    return {
      scope: {}
    };
  });

And here is a plinkr demonstrating the same: https://plnkr.co/edit/D53fDu1hwhvk5aKIsrJs
Edit: fixed paste error

[aluanhaddad]

@RyanCavanaugh to clarify what this does:
It registers two identically named directives, each of them specifying an isolate scope.
Angular actually registers both directives and the result of writing

<my-directive></my-directive>

is that angular will explode with the error:

Multiple directives [myDirective (module: app), myDirective (module: app)] asking for new/isolated scope on: <my-directive>

This is very bad!

[Cliveburr]

@aluanhaddad Yes, that does happen, I had to do a little helper to prevent it, not the way you showed, but the loading of routes in a SPA navigation, pages where is visited several times in the same session, but however, I believe that this effect is a framework problem or the loader mechanism problem, not the language itself ..

[RyanCavanaugh]

😭

[aluanhaddad]

@Cliveburr Of course it is not a language problem. It is also not a loader problem or a framework problem. The problem is incorrect code. If you are having issues like this it is because you have a bug in your code. There are so many ways to avoid this particular problem.
Here are two ways:

1. my-directive.ts

   export function myDirective() {
     return { .... };
   }

   app.ts

   import 'angular';
   import { myDirective } from './my-directive';
   
   angular
     .module('app', [])
     .directive('myDirective', myDirective);

2. my-directive.ts

   function myDirective() {
     return { .... };
   }
   export default function register(app : ng.IModule) {
     app.directive('myDirective', myDirective);
   }

   app.ts

   import 'angular';
   import registerMyDirective from './my-directive';
   
   const app = angular.module('app', []);
   
   registerMyDirective(app);

If you follow one of these patterns, you will not ever have this problem.

[markrendle]

@RyanCavanaugh wrote:

Are people seriously writing modules with non-idempotent load-time side effects? That seems insane

It's not insane, it's JavaScript. JavaScript modules have inline code that is executed when the module is loaded. It is the responsibility of the module loader to ensure that this only happens once. In my particular case, Webpack does a perfectly good job of this, but the same is true of RequireJS, Node/CommonJS, SystemJS, etc.

@aluanhaddad It is increasingly unclear to me what you think you are contributing to this issue. None of your comments have been constructive or helpful; the Plunkr you offered as a "repro" uses neither import statements nor module loading and is therefore entirely irrelevant in this context; the "workarounds" you have offered are either unusable in many contexts or horribly inelegant and fragile, and unintuitive to boot. Your assertion that "it only works precisely because TypeScript elides these imports" is flat-out wrong: I can and do repeatedly require() and import modules with side-effects throughout my application, and TypeScript does not elide the generated require statements, and it works because every JavaScript module loader in existence guarantees that the source file for a module will only be loaded and executed once.

Your obvious misinformation aside, may I propose that we take it as read that you are not encountering this problem yourself and therefore don't need the feature requested, and, in return, you accept that some people are and do, and that their experience is as valid and important as yours, and move on? Thank you.

[markrendle]

All I am asking for here is some kind of directive so I can say to TypeScript "hey, don't tree-shake this import, because I know my code-base better than you". Asserting that there are never going to be cases where tree-shaking should not occur is short-sighted and hubristic. Side-effects exist, are already well-handled in the JavaScript module ecosystem, and clearly in some cases are desirable, so TypeScript needs to recognise that.

Proposals

I can think of two directions from which to come at this. Firstly, from the side of the consumer of an import, such as this syntax (import-bang):

import! {MyService} from './myservice';

The second option is to declare within the module itself that it should always be imported, which would seem to make more sense, since it does not require special knowledge of module internals on the part of the consumer. Could be done with a /// comment or a free-standing string (like 'use strict';) within the module:

/// <ts-disable-elision>
export class Foo { }
angular.module('fubar').service('foo', Foo);

[Bart76]

@dawidgarus

Is it?

Yes, it is.

I would imagine that rules would be fairly simple. If module has only declarations of classes, interfaces, namespaces (with only classes and interfaces) and imports to module without side effects, then it doesn't have side effects.

Nope. That's too short-sighted. How about exported classes which have a static binding that is initialized by a static method that has side effects?

Example with four files, all placed in a webserver's "root" folder:

test1.ts:

export class ClassWithSideEffect {
  static someValue = ClassWithSideEffect.initValue();

  static initValue(): string {
    console.log("We have a side-effect here!");
    return "something";
  }
}

test2.ts:

//@ts-ignore
import { ClassWithSideEffect } from "./test1";

ClassWithSideEffect; // If you remove this, the import is removed by TSC and you will not get side effects!!!

export class OtherClass {
  // Todo: implement OtherClass. Or not. It's just a demo, so this class does not need an implementation.
}

tsconfig.json:

{
  "compilerOptions": {
    "target": "esnext"
  },
  "include": [
    "./*"
  ]
}

test.html:

<!DOCTYPE html>
<html>
<head>
  <meta charset="utf-8" />
  <title>Test</title>
  <script type="module" src="test2"></script>
</head>
<body>
  No content. Look at the console window in the browser's Developer Tools. It should show some "side effects".
</body>
</html>

Remove or comment out the second code line in test2.ts and compile. You will not get any side effects in the browser console any more. :(

I could result in some false positives, but don't think it would be a big dead. For the most uses it would work fine, and the rules could be refined in the future.

Invalid and wrong assumption! IMHO, it IS a "big dead" if it does not work correctly in all imaginable scenarios. It breaks my code! And my code is valid. I do not want to discuss this any further with you. I do not and will not agree with you.

As long as I can disable the logic altogether, you can refine your rules as much as you want.

Thanks for understanding.

[Bart76]

@dawidgarus

Please don't get me wrong. It is not my intention to agressively dismiss your contribution. We are all doing the best we can to make things better.

But we are talking about compilers here. They should work correctly. In all scenarios. There's nothing more important for software developers. That's why I am so direct and defensive about my point of view here.

[swar30]

one thing to take into account, lazy loading, what ever solution we have here next scenario should not load foo module

import {Bar} from 'foo';

export async function doSomeThing(): Bar {
  const foo = await import('foo');
  return foo.buildBar();
}

In this case, foo is lazy loaded, but in our code we want to use type Bar from foo and we expect type to be usable, while library itself not been injected into our bundle.

[shavyg2]

hi I had this issue bite me. It's a simple question i guess, is typescript a superset of javascript or does it aim to create some of it's own rules regardless.

I thought i was able to convert a code base of js slowly to ts without worrying because it's a super set? is that no longer the case?

[ohroy]

I had this issue too...I try 5 hours to found the decorators is useless for this reason.
wtf......

[karianpour]

So, any work around?!

[swar30]

@karianpour, to the original issue in this thread yes, just do double import once of the type and once just of the module with no types, look at the start of the thread for details

[whzx5byb]

I use export * from './MyService' where myservice.ts exports default like export default class MyService {}.
It doesn't reexport anything but it do force to execute the side-effect codes.

If you use barrel files(index.ts which re-exports module), every file must export an empty default value as below, to prevent tree-shaked by ts compiler.

index.ts

export * from './a';
export * from './b';   
export * from './c';
export default {}; // <- this is required

PS: I prefer export * from './MyService' rather than import './MyService' because the former reports a error path while the latter doesn't.

[viT-1]

In MainFilter.ts I use import { conf, IData } from './MainFilter.conf'; and in dev-mode for browser use <script type="module">
MainFilter.conf.ts has interface IData:

export interface IData {
	class: string;
	orderNumber: string;
}

In browser console I have an error:
Uncaught SyntaxError: The requested module './MainFilter.activity.conf.js' does not provide an export named 'IData'
cause of tsc cutting out interface export from MainFilter.conf.js =(
How to solve it? Why MainFilter.js not cutted out IData?

[viT-1]

Solved as

import { IData } from './MainFilter.conf';
import { conf } from './MainFilter.conf';

but forced to off import/no-duplicates rule =(

[andrewbranch]

FYI, I have a PR open to fix this at #35200. Feedback is welcome from anyone still interested in this issue. Thanks!

[lgarron]

This keeps tripping me up, so I thought I'd note it here.

This code breaks when the elem2 line is commented out:

import ClipboardCopyElement from "@github/clipboard-copy-element";

const elem1: ClipboardCopyElement = document.querySelector("clipboard-copy");
const elem2 = new ClipboardCopyElement();

Unfortunately, I find this is a pretty common situation when working with custom elements, and as a library author I'm frustrated that I can't fix this for library users. It also makes it trickier to recommend TypeScript to beginners. The only way to allow elem2 to be commentable in the code above is to change the imports to the following, which I think should be considered an anti-pattern (I would flag it in a code review):

import "@github/clipboard-copy-element";
import ClipboardCopyElement from "@github/clipboard-copy-element";

Given that TypeScript will probably treat import ClipboardCopyElement like import type ClipboardCopyElement in the foreseeable future, it would still be really useful to have "import with side effects" (ideally still compatible with normal JS syntax). This:

• could be used in foolproof sample code, and
• could still support some tree shaking, as pointed out at Should be possible to force import of "unused" modules in case there are side-effects #9191 (comment) .

(Ideally, I'd love to have tooling that encourages the use of import type when possible, and keeps ESM semantics for importing objects.)

[Bart76]

Lately my eye caught a compiler/tsconfig option called importsNotUsedAsValues when I was browsing the TSConfig Reference on the TypeScript website. Perhaps this issue here is solved when specifying the value preserve or error for that option?

[richardkazuomiller]

Sorry if I'm confused about why this is closed. This hasn't been fixed, has it?

It would be nice to have something that is the opposite of import type that tells TypeScript we want to actually import the file, even if the things we're importing just so happen to be types.

Right now, we have to do this:

import type { MyType } from './file';
import './file';

Is there a way to shorten this? If not, maybe something

import value { MyType } from './file';

import type indicates we only care about these imports in type space, and import value tells TypeScript that there is something in file.ts that we need in value space.

[jods4]

@richardkazuomiller I think you want to set the option importsNotUsedAsValues: preserve

With this, all your imports will be kept, only explicit import type will be removed.

[richardkazuomiller]

@jods4 Thanks, that looks like it might do the trick 😄