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

force import #596

Closed
alidorosty1234 opened this issue Sep 4, 2014 · 31 comments
Closed

force import #596

alidorosty1234 opened this issue Sep 4, 2014 · 31 comments
Labels
Question An issue which isn't directly actionable in code

Comments

@alidorosty1234
Copy link

I have many scenarios where I would do this:

import myExternalModule = require("./myExternalModule");
// not using myExternalModule here

I dont use myExternalModule in my code but still I want it to be included using requirejs. I just need it to be there.
If there could be a forceimport keyword that would be very cool!

@NoelAbrahams
Copy link

It should be possible to force the emit by writing

var myExternalModule = require("./myExternalModule");

but this won't complain if "./myExternalModule" is an incorrect path.

The correct approach is to introduce a dummy reference that has no side-effects:

import myExternalModule = require("./myExternalModule"); 
myExternalModule;

Although this is a hack, it's very unlikely that new syntax will be introduced for this rather narrow use-case. See the design goals.

@NoelAbrahams
Copy link

If TypeScript would verify the path in var myExternalModule = require("./myExternalModule"); then that would permit users to force the emit _and_ get compile-time verification, without having to resort to the hack.

@danquirk
Copy link
Member

danquirk commented Sep 4, 2014

The var approach doesn't import the types from the module though.

Additionally you could use the amd-dependency attribute if you must.

For what purpose are you importing things that aren't used?

@alidorosty1234
Copy link
Author

For instance in an angular app I have some directives in other files. I need to import those directives or else my html markup won't work. I don't use them in JavaScript though.
Usually in angular, most modules are decoupled and don't have dependencies on each other, but need to be imported for the whole app to work.

@vvakame
Copy link
Contributor

vvakame commented Sep 8, 2014

👍
It is confusing me.
Why TypeScript compiler eliminate it?
external module is not non-instantiated module.
require has a side effect.
import clause seems to be compatible with the require of AMD and CommonJS. but it is not.

@Bartvds
Copy link

Bartvds commented Sep 8, 2014

This issue is also annoying when require()-ing modules that don't really export anything but instead shim functionality on global objects, like es5-shim, es6-shim and many others.

@NoelAbrahams
Copy link

It is confusing me.
Why TypeScript compiler eliminate it?

It is an optimisation because sometimes we want to import only the type information from a module:

import foo = require('foo');

function bar(paramOne: foo.ParamOne, paramTwo: foo.ParamTwo){}

@mhegazy
Copy link
Contributor

mhegazy commented Sep 8, 2014

it is not just an optimization. If you are using an imported module for types only (i.e. not used in any value position), we can not emit the require logic for it, as the module can be an ambient type-only module that does not exist at runtime. Emitting a module import would mean a runtime error trying to load a non-existing module.

@RyanCavanaugh RyanCavanaugh added the Question An issue which isn't directly actionable in code label Sep 8, 2014
@RyanCavanaugh
Copy link
Member

You can use the amd-dependency flag for this:

/// <amd-dependency path="foo" />

import x = require('foo');

@nikhilk
Copy link

nikhilk commented Sep 12, 2014

Instead of <amd-dependency> which looks like a bit of a hack, it would have been interesting to use <reference name="..."> IMO.

@QueueHammer
Copy link

@danquirk It's an arbitrary and dangerous thing to have the TypeScript engine take on the responsibility of optimizing the import statements. There are many patterns that TypeScript does not yet capture from JavaScript. One example is how it does not address that this could have a type #229 .

Import is used to load dependencies before the current code is loaded and may not be referenced directly. Angular dependencies are one example, while jQuery plugins are another. Any library that extends a base object and is not referenced directly are affected by "feature". By deciding to arbitrarly not included an import based on local static analysis you are imposing a C style compiler pattern over the explicit intention of the developer who wrote the import statement. The expectation of writing import is that it will be included as a dependency and available to the local scope of the module. Any other action is a side effect to the natural expectation of choosing to write import inside a module.

It is very possible that in this case the TypeScript compiler could do less, and accomplish more.

@mhegazy
Copy link
Contributor

mhegazy commented Feb 25, 2015

TypeScript next release will support ES6 style modules (the change is currently in master). so if all what you want is declaring dependency you can use:

import "myLib";

The compiler will not elide this import.

@QueueHammer
Copy link

Will the existing inconsistent behavior be address, or remain as something fun for people to discover? Should it be added to the documentation that their is a case where import does not add the file as a dependency? When building TypeScript AMD modules will ///<reference... now be considered an error?

The way import currently works is bad design. Bad design creates ambiguity. If the additional behavior is not clearly defined then it will more than likely be discovered though it's side effect. Hence the OP, myself, and other comments and bugs related to it in this forum.

There is an existing body of practice that already exists when using Common or Require style dependency management. The implementation of import shows that it was ignored.

@mhegazy
Copy link
Contributor

mhegazy commented Feb 27, 2015

Regarding documentation, please refer to the spec section 11.2.5 and section 11.2.6:

An external import declaration is represented in the generated JavaScript as a variable initialized by a call to the 'require' function provided by the module system host. A variable declaration and 'require' call is emitted for a particular imported module only if the imported module, or a local alias (section 10.3) that references the imported module, is referenced as a PrimaryExpression somewhere in the body of the importing module. If an imported module is referenced only as a ModuleName or TypeQueryExpression, nothing is emitted.

/// references are not the same as imports. /// references brings additional declarations into your global scope, e.g. dom APIs. it is a statement to the compiler to trust the existence of these entities, and it does not have impact on the generated code.

Imports on the other hand need to be emitted as require statements. in the case you are only using an import in a type position, the intent is not clear, either you want only the types (which are design time constructs that do not exist at run time), and in this case you want the import elided, and if it is not you would be importing possibly a non existing type-only module. Or you want the side effects as well. For the latter, import "mod"; syntax seems to be a clearer syntax to declare intent.

@tomitrescak
Copy link

@mhegazy I do not want to open a new issue, but this is a bit problem in React world.
Consider the code below, when compiled to js the React reference is removed as it does not appear in the code and the code will fail as ReactRouter depends on it. The fix ... really dumb is below the code. Is there any smarter way of deoing that? Thanks.

import * as React from "react"; var temp = React.DOM;
....

// mf("route.schedules", "") // this is to register a translation
anonymousRoutes.route("/schedules", {
  name: "schedules",
  subscriptions: function() {
    this.register("schedules", subscriptions.subscribe("schedules"));
  },
  action: () => {
    ReactLayout.render(ClearLayout, {content: <Book />});
  }
});

FIX ;) - EHM

import * as React from "react"; var temp = React.DOM;
...

@mhegazy
Copy link
Contributor

mhegazy commented Sep 15, 2015

the code will fail as ReactRouter depends on it.

does it need the var "React" or just some side effects from the import?

@tomitrescak
Copy link

It need the side effect from the import. The var was added to only not remove the import in the transpiled code.

@mhegazy
Copy link
Contributor

mhegazy commented Sep 15, 2015

so why not just add

import * as React from "react"; // get the types
import "react";  // just for side effect

in the output, the last import: import "react" will not be elided.

@tomitrescak
Copy link

Unfortunatelly this dows not work and I still get:

ReferenceError: React is not defined
    at action [as _action] (schedule_router.jsx:15)
    at Route.callAction (kadira_flow-router.js?f961d732ed2b89a53d490af5979b899800aa1a5d:2306)
    at kadira_flow-router.js?f961d732ed2b89a53d490af5979b899800aa1a5d:2025
    at Object.Tracker.nonreactive (tracker.js:560)
    at kadira_flow-router.js?f961d732ed2b89a53d490af5979b899800aa1a5d:2016
    at Tracker.Computation._compute (tracker.js:294)
    at Tracker.Computation._recompute (tracker.js:313)
    at Object.Tracker._runFlush (tracker.js:452)
    at Object.Tracker.flush (tracker.js:412)
    at Router._invalidateTracker (kadira_flow-router.js?f961d732ed2b89a53d490af5979b899800aa1a5d:2065)

I think that the reason here are the "weak dependencies" in Meteor, which depend on callee to provide all the necessary references for them.

@scottwio
Copy link

I've started to learn typescript recently for Angular 2. This has caught me out a number of times as I'm sure it will for other new comers. I understand that this is an optimisation but it's actually very confusing. If I type something in my ts file I expect it to produce, output or some sort not simply ignore it. I get that I can force the require statement to be produced but that seems little a hacky. This seems like a cause where I as the user should be able to make my own decisions with out the compiler deciding what's best.

@mhegazy
Copy link
Contributor

mhegazy commented Sep 21, 2015

TypeScript overlays type declaration on top of values, so the same import syntax gets you types, values, or both. this makes it very convenient and intuitive to work with module; if you need a type from another module, just import it as you import a variable, or just import the class, and use it both as a type and as a constructor with new.

The flip side of this, you can have imports to modules that do not exist, as they are just type containers. if the compiler emitted references to these modules, it is going to fail at run-time. the solution we have in place is to detect how the import was used, and all references to it are used as types, then it is not needed at run-time and elided.

obviously this can be confusing if you need the side effects of a module as well. but the other way around is confusing as well if an import to a non existing module was emitted, let a side having to loose the convenience of bundling types and values together in the same syntax.

@weswigham
Copy link
Member

A workaround:
If using traditional resolution (moduleResolution is not set to node), an import statement containing a ! in its identifier will never be eluded. I believe this has something to do with us trying to support some systemjs behavior better, but can be used to force an import. If you are using systemjs, requirejs, or any loader which allows remapping of module identifiers, you can end your forced import in !js or a similar mark and map it away with your module loader. TS will emit the import verbatim (and not try to typecheck or resolve it), and your loader can be taught how to understand the import.

@ChristopherElliott
Copy link

It appears the team is already having to make special-case code for these types of imports such as in checker.ts:

// If we're compiling under --jsx react, the symbol 'React' should
// be marked as 'used' so we don't incorrectly elide its import. And if there
// is no 'React' symbol in scope, we should issue an error.
if (compilerOptions.jsx === JsxEmit.React) {
    let reactSym = resolveName(node.tagName, "React", SymbolFlags.Value, Diagnostics.Cannot_find_name_0, "React");
    if (reactSym) {
        getSymbolLinks(reactSym).referenced = true;
    }
}

As @mhegazy points out this isn't just about optimization but rather it is to avoid requiring an import with no runtime definition (e.g. Interface) which would cause a runtime error.

I question which will happen more often. You could easily detect if the import is a *.d.ts so those would be easily elided.

In the rest of the cases it seems like a pretty trivial thing for the compiler to detect that the imported module would have no runtime output and only elide those imports.

From a DRY perspective, this suggestion left me wishing I had a transpiler for your transpiler to write this duplicated code for me.

import * as React from "react"; // get the types
import "react";  // just for side effect

@TheWizz
Copy link

TheWizz commented Oct 29, 2015

The way I see it, it should be the imported file's choice whether it needs to be loaded at runtime (because it knows it has side effects). This really should not be decided by the loading file (i.e., the file with the import statement), since it's not its business to know implementation details of other files/modules.

At least this would work well from an angularjs perspective, where a file that defines directives and services "knows" that it must be loaded at runtime if referenced, so the compiler must not elide it. Some kind of hint in a file to the typescript compiler to "not elide" would fix this IMO. But I'm probably overlooking some use case here.

-JM

@laurelnaiad
Copy link

xmodule.ts:

console.log('ive been imported');
export class X {
}
import {X} from "xmodule"; // get the types
import "xmodule";  // just for side effect

If xmodule has statements, .e.g. the console.log statement, it is by definition not just a pile of declarations. It should not be the responsibility of the importing module to make statements which use xmodule in order for xmodule to be considered more than just a series of type declarations.

@micah-williamson
Copy link

@weswigham

You mentioned using a bang to force your import. That is preferred for my scenario. How do I use it though? Tried all of the following:

!import {Service} from './service';
import! {Service} from './service';
import {!Service} from './service';
import {Service!} from './service';
import {Service} from '!./service';
import {Service} from './service!';
import {Service} from !'./service';
import {Service} from './service'!;

@weswigham
Copy link
Member

It was in the string - but I think the behavior was removed recently when path mappings were introduced.

@idchlife
Copy link

I'm using library, like react, for importing node wrapper - h (in react it's react() if I'm not mistaken), so I import like this:

import { h, Component } from "preact";

Then, of course, I have no h in the runtime. Because why would in be there? Also, using with jsx preserve, so TypeScript really does not about h, but babel then knows.

So, what is left, using reference like h; after import, yes?

@mhegazy
Copy link
Contributor

mhegazy commented Nov 13, 2016

For h (or any other custom jsx factory) use --jsxFactory to let the compiler know that this is the factory you are using at runtime. e.g. --jsxFactory h.

Not this requires typescript@next at the moment, should be available in the TypeScript 2.1.2.

@michael8090
Copy link

michael8090 commented Feb 20, 2017

@mhegazy For projects that have two kind of JSX consumer (such as react and snabbdom) it's not acceptable.

I'm working on a project that uses react to render web UI and a customized virtual dom implemented by ourselves to render webgl objects. And it hits the corner case, as we NEED two different kind of @jsx annotation for the same project.

Now I'm forced to export our own h as a global variable... It's ugly.

@yavin5
Copy link

yavin5 commented Apr 21, 2017

Include me in the list of developers who need/want a one liner import that does this. Two lines of source to accomplish it is what we're trying to eliminate, especially when we want to import many modules in a row. I guess what sounded best so far to me is a keyword / syntax to force the import. Thanks again guys!

@microsoft microsoft locked and limited conversation to collaborators Jun 18, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Question An issue which isn't directly actionable in code
Projects
None yet
Development

No branches or pull requests