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.
It's possible:
import './MyService`;
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.
:)
I am too dumb for getting why import {MyService} from './MyService';
would not work, please enlighten.
please see https://github.com/Microsoft/TypeScript/wiki/FAQ#why-are-imports-being-elided-in-my-emit
@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.
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)?
@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"
@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...
@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.
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.
@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);
}
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';
!
is already treated specially as part of AMD plugin paths
@RyanCavanaugh Good thinking!
@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.
Are people seriously writing modules with non-idempotent load-time side effects? That seems insane
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
@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!
@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 ..
:sob:
@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:
TypeScript
export function myDirective() {
return { .... };
}
_app.ts_
``` TypeScript
import 'angular';
import { myDirective } from './my-directive';
angular
.module('app', [])
.directive('myDirective', myDirective);
```
TypeScript
function myDirective() {
return { .... };
}
export default function register(app : ng.IModule) {
app.directive('myDirective', myDirective);
}
_app.ts_
``` TypeScript
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.
@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.
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.
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);
Why is non-compliant ES6 syntax better than the following again?
import './MyService';
import {MyService} from './MyService';
Ugly is clearly in the eye of the beholder.
@kitsonk Multiple points:
5) I'm not sure what "non-compliant ES6 syntax" means, nor what it has to do with a request for a TypeScript feature.
From TypeScript's design goals:
6) Align with current and future ECMAScript proposals.
8) Avoid adding expression-level syntax.
So TypeScript tries to align to current and future ES Syntax, so it is material to any feature request.
The syntax:
import! {MyService} from './myservice';
Would have to be rewritten in all instances to generate a compatible ES Module. The elided import can just be erased, with no re-writes and without introducing expression level syntax.
Doesn't this proposal also suffer from your item 4) which the burden of knowledge is on the consumer?
While the triple slash comment does not add any expression level of syntax that is syntactically meaningful to ES.
Again "ugly" is still in the eye of the beholder. I don't think either of your solutions really improve about item 3) of your list. I don't think import!
is clear at all...
I personally am not uncomfortable with the current solution, but if you wanted to really propose introducing some sort syntax to express this, why not propose something that doesn't use cryptic syntax:
import always {MyService} from './myservice';
/* or */
always import {MyService} from './myservice';
I suspect the former would be better (since the ES import syntax is largely backwards anyways).
Placing some form of directive in the module to be forced to load addresses points 1 through 4 (and probably 5). The change happens in a single file, instead of wherever that file is referenced, the import statements in the consuming files are ES6 standard, and the ///
directive approach will, as you say, be ignored as a comment by ES6. Furthermore, if an update is made to the module that no longer requires it to be force-loaded, only the module needs to change.
So, the second suggested approach obviously makes more sense.
Can I formally propose some triple-slash-based directive along the lines of:
/// <ts-always-load>
export class StockService { }
angular.module('shop').service('stock', StockService);
Behaviour: When the TypeScript compiler encounters this directive in an import
ed module file, it will always generate the equivalent require
, or leave the import
intact, even if the imported members are not used in the currently-compiling file.
Disclaimer: I am not, as you can clearly see, a language designer, so I have no idea how to turn this into a proper formal proposal. Sorry.
How do you plan to handle a situation where the developer is actually only importing an interface/class for its shape but expects/wants any side effects to not occur? Not every module that can have side effects is some sort of form of DI. For example, let's say I have a shim library that is exposing an API surface, but I want/need to intelligently load that code in a different way, and not eliding the module causes issues or inefficiencies at run-time?
Also, what effect does this proposal have on the emit of definition files (.d.ts
) and how does that behave?
Why would a shim library, or a module that is to be loaded in a non-standard way, have the always-import
directive? Can you give a realistic example?
I can't imagine any effects on definition file emission, or changes that would be required in that area.
Why would a shim library, or a module that is to be loaded in a non-standard way, have the
always-import
directive? Can you give a realistic example?
Let's say I have a Promise
shim. I want to import the API surface:
import Promise from 'my-promise-shim';
function logPromise(p: Promise<string>) {
p.then((value) => console.log(value);
}
But at build time, I may or may not want to elide my-promise-shim
if I am targeting and environment that supports them natively, but it is likely the shim then would be included in the output, because it will be rewritten as an import and then not elided in the final build, even though I don't want it there.
There are issues already open for conditional compilation and it almost seems like solving this potentially would fall under that, as I think it is hard to properly express a "you must always include this", because that is sometimes not that black or white.
I can't imagine any effects on definition file emission, or changes that would be required in that area.
What happens if I am consuming a built package of JavaScript code that contains modules with side effects and in TypeScript land it is described using a definition file? How would TypeScript know to elide or not elide? Especially when not using UMD definition files, but a global definition with namespaces.
I'm still not clear on why the Promise
shim, given its optional nature, would have the always-import
directive.
TypeScript doesn't currently elide requires from definition files if the imported symbol is used other than for type declaration, otherwise it does. I am not proposing that the always-import
directive be added to .d.ts
files.
I've been tripped by this issue as well when using RxJS.
import { Observable } from 'rxjs';
...//omitted code
get<TResult>(url: string, params: Param[])
{
let urlParams = new URLSearchParams();
this.serializeParamsForQuery(urlParams, params);
return this.http.get(url, { search: urlParams })
.map(r => r.json() as TResult);
}
I couldn't figure out why it didn't work, even though I use the map function.
Using import 'rxjs';
fixed the issue but this doesn't feel right.
Typescript is trying to be clever but shoots itself in the foot. IMO optimizations like this should only be implemented if they work 100% of the time, otherwise the compile should err on the side of caution and be conservative.
TypeScript design goals:
Emit clean, idiomatic, recognizable JavaScript code.
Preserve runtime behavior of all JavaScript code.
Something new about this functionality?
@clement911 no it's not being clever, it's removing TypeScript artifacts from the source. Your snippet doesn't even use what you imported. However, if you had used it, RxJS is structured in a very confusing manner such that the map function is not a member of a Observable.prototype. You need to add an additional import
import 'rxjs/add/operator/map';
and even if you use Observable
as a value, as in Observable.of(1).map(f)
, in which case TypeScript does retain the import because, if you import {Observable} from 'rxjs/Observable'
map will still throw an error. This is a compile time error in recent versions but the fix is not obvious. However if TypeScript retained the import used only in type position it would not be correct. It would not be removing TypeScript related artifacts from the source.
Note to the side effecting import above actually mutates Observable.prototype
. It's a weird pattern but it's good that it happens explicitly as otherwise using it simply for type annotations would change the emit which would in turn cause imports in other modules to break because the import would be evaluated and cached before its prototype were mutated.
I read all the replies and I still hope this is not an already given answer to the original question:
What you can do is put the code in MyService.ts in a namespace and create a interface for MyService: IMyService. The only code you 'export' in the MyService.ts is the interface:
import {ngModule} from './../module.ts';
namespace myAwsomeNamespace {
export interface IMyService{
myVariable: string;
}
class MyService implements myAwsomeNamespace.IMyService {
public myVariable: string;
constructor() {
this.myVariable = 'hello world!'
}
}
ngModule.service('myService', MyService);
}
import './MyService.ts';
class MyComponent {
constructor(private myService: myAwsomeNamespace.IMyService) { }
// ...
}
this let you import/load 'side-effects' like you would nomally do and make the injected service type-safe. The other benefit is this example is that the MyService class is not put on the JavaScript Window object.
Look at the output of the TypeScript transpiler:
define(["require", "exports", "./../module.ts"], function (require, exports, module_ts_1) {
"use strict";
var myAwsomeNamespace;
(function (myAwsomeNamespace) {
var MyService = (function () {
function MyService() {
this.myVariable = 'hello world!';
}
return MyService;
}());
module_ts_1.ngModule.service('myService', MyService);
})(myAwsomeNamespace || (myAwsomeNamespace = {}));
});
RxJS is a weird example in that what you're importing is a set of "extension methods" to Observable, i.e. they have no standalone representation like classes or functions.
In my most recent Angular 1.6 code I switched to a different pattern based in John Papa's and Todd Motto's styleguides, which I'll lay out here for posterity.
Each component and related assets is contained in its own folder, along with a module file. A Component class looks something like:
import { IHttpService } from 'angular';
export class LoginFormCmp {
// @ngInject
constructor (private $http: IHttpService) { }
...
public static ngName = 'loginForm';
public static ngComponent = {
bindings: { ... },
controller: LoginFormCmp,
template: require('./loginForm.html')
}
}
The module file then imports the component class and uses the static members, so the import is not elided:
import * as angular from 'angular';
import { LoginFormCmp } from './loginForm.component.ts';
const ngModule = angular.module('app.loginForm', []);
const ngName = ngModule.name;
ngModule.component(LoginFormCmp.ngName, LoginFormCmp.ngComponent);
export default ngName;
Obviously a service looks something like
export class DogmataService {
// ...
public static ngName = 'dogmata';
}
It's actually a nice way of organising code generally, as it encourages more separation and isolation, and smaller components.
But there should still be a way of stopping imports from being elided when there are desired side-effects, because even if it is not recommended practice
, it happens all over the place, and things should be built for the world we have, not the one we may wish for.
In my most recent Angular 1.6 code I switched to a different pattern based in John Papa's and Todd Motto's styleguides, which I'll lay out here for posterity.
Thank you, I didn't know the styleguide from Todd Motto. The way that John Papa explaines the registering of angular modules, services, directives etc. is a bit different.
Where Todd is registering all the angular things beforehand, John Papa registers the angular things as you define them: (link)
/* recommended */
// dashboard.js
angular
.module('app')
.controller('DashboardController', DashboardController);
function DashboardController() { }
But there should still be a way of stopping imports from being elided when there are desired side-effects,
When we were developing in JS instead of TS we used the styleguide from John Papa. Since a few months we are developing in TS.
Why I placed the quotes around 'side-effect' is because defining a service and then register it with Angular is in my opinion not really a side-effect. The service is not doing anything until I call it from where it is injected (unless developed otherwise).
What I like about the 'side-effect' method (on-demand referencing) is that you only reference a file when you need it. Like you do with usings in C# or Java. On the other hand, the application structure needs to compensate more in what the on-demand referencing lacks..
I've been bitten by this numerous times. To me, it seems it should be the _imported_ module that dictates whether it should always be loaded (for side effects), not the _importing_ module (which has less knowledge about the innards of the imported module). Is there really any case where it makes sense for the _importing_ module to control this behaviour?
-JM
Hi, all, I vote for this feature.
My proposal is to add some flag "noOptimizeImports" to compiler options (tsconfig.conf, etc.) and _not to add new syntax_ for language.
This is very important to modules which are plugins to something else. For example code:
import {Product} from "./product";
export class MyPlugin {
...
}
Product.addPlugin(new MyPlugin());
currently will not be included to the product if I write:
import {Product} from "./product";
import {MyPlugin} from "./myplugin";
console.log("MyPlugin version = " + MyPlugin.version);
Product.run();
I think TypeScript must follow what developer says and do not do none-obvious things.
@TheWizz Yes, there is a case. If you use a DI system like @molecuel/di :-). I am tagging classes with @injectable to mark them. Just a import is needed to load them into the DI.
I had the Problem that it did not work cause i only had imports for some classes in the main file without using them there. I created a little workaround and do a di.bootstrap() and add the imports as parameters.
I am running into the same issue in a very valid context.
When using decorators on a getter like this:
import { ADecorator } from './a-module';
import { SomeClass } from './some-module';
class MyClass {
@ADecorator()
public set myProp(value: SomeClass) {
//do stuff
}
}
Here the TypeScript compiler outputs something like this (when using commonjs modules):
__decorate([
amodule_1.ADecorator (),
__metadata("design:type", some_module_1.SomeClass),
__metadata("design:paramtypes", [some_module_1.SomeClass])
], MyClass .prototype, "myProp", null);
The problem is that since I didn't use the "SomeClass" type (other than for type info) the module does not get imported and the code breaks at runtime.
that last thing looks like a #12888 which should already be fixed in nightly build. Can you please try typescript@next
to see if you still see this problem?
Using version 2.3.0-dev.20170221 did not seem to solve the problem. But my build process/tooling is kind of complex so I'll have to play with it and see what's happening.
For now I am solving the problem by just adding an erroneous usage of the offending classes.
@TheWizz
To me, it seems it should be the imported module that dictates whether it should always be loaded (for side effects), not the importing module (which has less knowledge about the innards of the imported module). Is there really any case where it makes sense for the importing module to control this behaviour?
It was my gut feeling as well: modules should declare if they have side-effects or not, not consumers.
And then I met someone who was importing types from a module because he needed the _types_... but did not want to initialize the module yet. The details why are a little bit convoluted but it comes down to having to load another module with side-effects first (yay for side-effects all over the place).
And now I'm confused as to what is the best solution.
The best solution is just to write
import 'module-specifier';
import {SomeType} from 'module-specifier';
It is extremely clear as to intent, is an existing ECMAScript import form (one specifically created to import modules for their side-effects) and your loader will understand it immediately. I do not see why the extra line is so unappealing to many.
Yes, that's what I do now. It just feels "backwards" that tne IMPORTING module should know it needs to do this in order to use another module. The IMPORTED module should have a better idea about whether it has side effects or not, and should be able to "do the right thing" without being told by a client. I understand there may be some edge cases where this doesn't hold. But in all practivcal cases where I've stumbled over this, it has been entirely clear to me that if I import something frmo a module I *definitely" want that module to be brought in at runtime (and not just "peeked" at at compile time).
-JM
@aluanhaddad,
This is not only "one more line" you need to write.
This is a pitfall you can fall into: TypeScript was designed to avoid mistakes in compile-time, but with current behavior these situations go in run-time. Sometimes go and sometimes don't. Sometimes you need such line, sometimes not. Do you think this is fine?
I do acknowledge the issue that you might want to import a module _only_ for the types, and not the side effects. I feel that most cases where you want to import a module for the types, however, you would also want the side effects. And selfishly, especially with AngularJS, those side effects are often necessary to the running of the application.
In our scenario specifically, we are using Webpack together with AngularJS to lazy load controllers, and other injectables (components, directives, filters, and services). If TypeScript elides certain imports our whole dependency tree falls apart. In any case I would want babel to do the tree shaking, not TypeScript.
This makes me raise my hand in supports for @dmiko's proposal to have a noOptimizeImports
compiler option that would prevent TypeScript from doing tree-shaking.
Alternatively, either the proposal to allow a module to be annotated that it has side effects as suggested by @markrendle, or the import always 'MyModule';
syntax would work as well.
The suggestions that we either double the imports for modules with side effects, or "use" the imported item, will both cause problems. Both look like mistakes that some good samaritan will undo, resulting in bugs. That much I can say for certain.
@aluanhaddad I think that the code you say is clear in intent is in fact confusing and bloated. Only those in the loop will know why it's a two-liner, and I have to worry about ensuring none of my developers do some clean-up or deal with occasionally forgetting to put in this extra token line of code when it really shouldn't be necessary.
The reality I think that should be accepted is that TypeScript's tree-shaking approach makes assumptions about codebases that should not necessarily hold true the entire time.
I've just ported an angular 1 app to ES6 modules. My initialisation file and process for extending the codebase could be literally halved in complexity if a 'force/always' keyword was introduced.
Another little example of why this is needed:
I have to declare this useless array just to include them... then I have to just say the array again to stop any unused variable errors.
After a lot of headbutting... this is what some of the code looks like. Emphasis on 'some'.
That block of code goes for another 40-50 lines, and looks like an absolute eyesore. My initialisation script is big enough without having to deal with this issue. A decision on this would be appreciated.
Having same issues and same problems, some kind of resolution here would be greatly appreciated
Me also, hitting similar problems during Inversify injection
But why do we have module elision there in the first place? TypeScript promises to be as-ES-compliant-as-possible, but module elision is something going very against this philosophy. I've personally met the same problem as in https://github.com/Microsoft/TypeScript/issues/18586 (https://github.com/pugjs/babel-plugin-transform-react-pug/issues/55), and this "optimization" trick is really hateful that it doesn't work with my favourite language of use.
TypeScript promises to be as-ES-compliant-as-possible, but module elision is something going very against this philosophy.
The principle is based on design goal:
Therefore when TypeScript believes a construct is only used for the type system, it erases it, so that the emitted JavaScript has what TypeScript believes is the right behaviour. If it were to leave it, it could contravene the other design goal:
So while it _may_ make sense to find a way to indicate to TypeScript that an import has run-time side effects that cannot be detected by TypeScript, just leaving them is not a good idea.
@kitsonk I know it has been debated to death, but that "indication" to TS that it should not erase the dependency could save a lot of ugly dependency duplication for AngularJS and possibly other DI based systems.
We are running in to the same issue. We have a legacy js library for which we are coming up with d.ts files. Our webcomponents does not return anything and when 'require'd, they just registers the components with DOM.
So for these modules all we want in our definition files is a 'no side effect' type-checking module to validate component properties and methods. And by importing this module like,
import {Component1} from 'Component1Module' works great for the type checking purposes. But since this import is dropped from the emitted js, our users will be forced to have another import
import from 'Component1Module' , which looks ugly and gross. More over it is error prone given the fact that the first import will compile the TS code. The lack of second import will only be caught in the runtime.
Is there any chance you can re consider this proposal?
I raised #28285, which is a similar issue, but when using ESNext modules in the JS output.
While I understand that finding out whether a referenced module has side effects is just about impossible to do reliably and a new syntax to force this is difficult to design, for ESNext modules we kind of already have the distinction _because we have to include the extension_:
// gets definition from component-definition(.d).ts - don't include in JS output
import { ComponentDefinition } from './component-definition';
// get definition from side-effect(.d).ts, JS loads side-effect.js
import { SideEffect } from './side-effect.js';
This fix for this sub-case would be (relatively) simple - if a .js
(or .mjs
) extension is present then _always_ include the import
statement in the JS output.
There may be a risk of losing the tree shaking, but the current workaround for this issue is to include both, so;
import './side-effect.js'; // Needed in JS output
import { SideEffect } from './side-effect'; // Definition for TS checking
This means that any tree shaking is lost anyway.
If I have types.ts
that exports bunch of types:
export type Some = {
[prop: string]: string;
};
and index.ts
that uses those types:
import {Some} from './types';
...
and I use rollup
with rollup-plugin-babel
with @babel/preset-typescript
to strip out types and build a js bundle, then seems like babel removes type export declaration in types.ts
, but doesn't remove type import in index.ts
, so rollup complains:
src/index.ts → dist/index.umd.js...
[!] Error: 'Some' is not exported by src/types.ts
In Flow it was easy, because type imports had a special syntax: `import type {Some} from './types'. But how to do the same in Typescript to make Babel remove such imports?
To start, I didn't read the entire thread prior to posting this message. Sorry about that, but this entire discussion is frustrating to me anyway, regardless of what everybody writes about it. I have read some reactions, however, and in several respects I didn't like what I read. This comment is addressed to the entire Angular and TypeScript development teams. Have fun.
I do not live in an Angular world. I live in a vanilla-ES6-with-HTML-web-components world.
HTML web component instances can be created by calling document.createElement()
or using textual DOM changes (using innerHTML
/ innerText
properties). In those cases, the component's underlying class' constructor is not explicitly called in the code, but will be called internally by the JavaScript interpreter/engine.
I personally like(d) to define my HTML web components in TypeScript files by first creating an exported class for the component's behavior, and after the class definition, within the same file, I include a call to customElements.define() to "register" the web component. So all my web component modules have side-effects! And this seems to be a natural (logical as well as intuitive) way of programming to me...
I have perfectly valid TypeScript files that are translated to perfectly broken JavaScript files due to the assumption that modules should not have any side effects. Why? Why has that assumption ever been made? To use Charles Babbage's words: I am not able rightly to apprehend this kind of confusion of ideas.
This seems to be a very nice example of a "smart solution" that attempts to optimize some "unnecessary" dependencies, but which now backfires, because it does a terrible job being not smart enough... As I see it, the TSC compiler generates a wrong result based on a correct input, while it thinks it generates a correct result based on a wrong input.
You might also argue that this "smart solution" attempts to correct "mistakes" from "naive" developers, who _might_ use unnecessary type declarations. When I personally use a specific type declaration in TypeScript, I really need it, because I will get compiler errors otherwise. In other words: when I use a type declaration, I need the type. Period. And I am quite sure that I am not the only one who thinks/works this way. So then who would be "naive" here: the TypeScript/Angular development teams, or a large part of the global development community?
So, in conclusion: to let things work correctly with the TypeScript compiler, I have to pollute my beautiful code in at least one of two ways:
Hmmmmm... Should I really elaborate on what I think about this?
Anyway, basically, I am a little screwed. Apart from being forced to rape my code, I have to manually review and analyse my complete TypeScript library. Do you know why? Because the f*cking TypeScript compiler does not inform me about where it has removed such "superfluous" imports!
Well... perhaps the compiler can inform me if I can set another verbose level or something like that, but I will have to look into that first. But to be honest, I would expect to _always_ be informed by the TypeScript compiler if it removes _any_ code in the emitted output that would normally be perfectly valid JavaScript code...
:-(
Suggestions to the TypeScript dev team:
(And in case you don't agree with my suggestions, would it be OK with you if your bank would "optimize" your bank account in a likewise fashion, for example by smartly detecting periodic payments to 3rd parties and automatically execute such payments without your consent?)
Thank you.
This should definitely be an option now given that there's now import()
types. If someone in this mode wants to import a module purely for its types they can do type Foo = import("./module").Foo
(or similar).
It think the compiler should detect whether the module has side effects and elide only modules which don't any have side effects. For example:
foo.ts:
export class Foo {}
Foo.prototype.foo = function() {}; // side effect
bar.ts:
export class Bar {}
index.ts:
import { Foo } from './foo';
import { Bar } from './bar';
function test(foo: Foo, bar: Bar) {}
Compiler should generate index.js:
import './foo';
function test(foo, bar) {}
EDIT:
This solution provides also an easy way to disable module removal in specific files, just add a dummy expression, like:
export class Bar {}
[]; // side effect
It will be probably removed by minifier anyway.
@dawidgarus
I do _NOT_ agree.
An analysis if a module has side effects is probably too complex. And even if such logic gets implemented correctly (and completely!), it will probably have a dramatic effect on the compiler's performance.
IMHO, there is no technical/AI replacement for neither developer stupidity nor developer ingenuity. When I include a module in TypeScript, I want it _ALWAYS_ to be included in the emitted code as well. It should not make assumptions about my programming skills. That's _MY_ responsibility.
I expect a compiler/transpiler to be straightforward. If I wanted an "optimizer", I would look for it in a separate application. Ever heard of the single responsibility principle, represented by the first letter of the SOLID principles? IMHO, this entire discussion is about a major design flaw. Period.
So as I see it, this "smart" functionality as it currently is implemented should at least be completely disableable with a configuration setting. Especially if it would become so "smart" as you propose. Such a configuration switch is probably the only simple and clear solution that will make everybody happy.
And - as I also said in my previous post - I would like the compiler to _ALWAYS_ inform me in its output/logging that it tried to do something "smarter" than just compiling/transpiling. (I am wondering how many more "smart" things the TypeScript compiler tries to perform...)
An analysis if a module has side effects is probably too complex.
Is it? 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.
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.
But I can see how it could be troubling for some.
An alternative approach could be leaving imports like this:
import {} from './foo';
import {} from './bar';
function test(foo, bar) {}
and let the bundler or module loader deal with it however you want.
Why implement some complicated logic when all we need is to not remove the import (by configuration)?
A C compiler doesn't remove "unused" #include
s. A C# compiler doesn't remove unused using
s, it actually reports an error, when the namespace can't be resolved. So the developer has to take care of them.
And in typescript the develop should take care of the imports.
Javascript should be valid TypeScript - in theory you should be able to compile Javascript files and they should not change in behavior. But removing imports does alter the code.
@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.
@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.
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.
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?
I had this issue too...I try 5 hours to found the decorators is useless for this reason.
wtf......
So, any work around?!
@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
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.
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?
Solved as
import { IData } from './MainFilter.conf';
import { conf } from './MainFilter.conf';
but forced to off import/no-duplicates rule =(
FYI, I have a PR open to fix this at #35200. Feedback is welcome from anyone still interested in this issue. Thanks!
Most helpful comment
It's possible: