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

--module=ES2016 should re-write "index" imports #12597

Closed
alexeagle opened this issue Dec 1, 2016 · 7 comments
Closed

--module=ES2016 should re-write "index" imports #12597

alexeagle opened this issue Dec 1, 2016 · 7 comments
Labels
Out of Scope This idea sits outside of the TypeScript language design constraints Suggestion An idea for TypeScript Too Complex An issue which adding support for may be too complex for the value it adds

Comments

@alexeagle
Copy link
Contributor

TypeScript allows shorthand for importing the index.ts file in a package. CommonJS does too.
However, this is not specified in ES2015 modules. Some loaders/bundlers support it (Rollup, Webpack) but some do not (Closure Compiler)

It would be more conservative to normalize these import locations to include the explicit /index suffix for better compatibility.
Repro:

$ ./node_modules/.bin/tsc -v
Version 2.0.10
$ cat import_index.ts
import {a} from './one';
console.log(a);
$ cat one/index.ts
export const a = 1;
$ cat tsconfig.json
{
    "compilerOptions": {
        "target": "es5",
        "experimentalDecorators": true,
        "noImplicitAny": true,
        "strictNullChecks": true,
        "moduleResolution": "node",
        "module": "es6",
        "rootDir": "",
        "declaration": true,
        "lib": ["es6", "dom"],
        "baseUrl": ".",
        "types": []
    },
    "files": ["import_index.ts",
    "node_modules/@types/node/index.d.ts"]
}
$ ./node_modules/.bin/tsc
$ cat import_index.js
import { a } from './one';
console.log(a);

The proposal is this should have been written as import { a } from './one/index';

@aluanhaddad
Copy link
Contributor

Just my 2 cents, and I do not mean to be rude, but I think people should stop using this pattern. It can be ambiguous, it is not supported by the emerging loader specification or SystemJS which seeks to model it, and it is not an obviously great pattern. It is more of a node convention. The other reason to avoid it is that it could well change the meaning of existing code.

@evmar
Copy link
Contributor

evmar commented Dec 1, 2016

@aluanhaddad I also really dislike it, but unfortunately we're trying to support the language that TS supports.

@ajklein What's the status on browsers supporting these sorts of imports?

@mhegazy
Copy link
Contributor

mhegazy commented Dec 1, 2016

The position we have taken is that module identifiers are strings that users wrote, and we do not mess up with them. no rewriting, no fixing. whatever you write there will be in the output.

The compiler should be able to handle you setting a full path in your import. e.g. import { a } from './one/index.js'; if you so choose. it will still know that you mean to import the ts file ./one/index.ts.

We could add a new module resolution strategy, call it ES2015 that does not do the extra node resolution steps for packages. and thus give you an error if you try to import .\one\index.js through .\one, but that can be achieved today by a tslint rule that says all imports need to have a .js extension at the end.

@alexeagle
Copy link
Contributor Author

alexeagle commented Dec 1, 2016 via email

@mhegazy
Copy link
Contributor

mhegazy commented Dec 1, 2016

In general, TypeScript accepts TypeScript semantics and lowers them to the
accepted syntax and semantics of the target. How hard would we need to work
to convince you that module identifiers might be included here?

module systems have different semantics when it comes to module names. for instance loder extensions. they have different syntax between AMD, and SystemJS for instance (json!./resource vs ./resource!json). other things related to configurations, and the underlying module system.
Some webpack extensions rewrite module names, and babel extensions that rewrite module names. Add JSPM on top and you have a whole new beast. and the list goes on..

the whole module resolution philosophy here is, you have some JS code that works with some tool. the compiler does not need to understand what it is, or where it comes from, all it needs is to know where to find a declaration file for it. hence the ambient module declaration (declare module "blah" ...) construct.

Doing this correctly all the way, will need the compiler to always understand what you are writing in the module name as a file location (something we do not do today), moreover, it breaks the single file emit case, where the compiler does no resolution or locating of files on disk.

@ajklein
Copy link

ajklein commented Dec 2, 2016

Looping in @domenic to help answer @evmar's question about browser support for this kind of resolution, as I know he's been involved in some design work in this area.

@mhegazy mhegazy added Suggestion An idea for TypeScript Too Complex An issue which adding support for may be too complex for the value it adds Out of Scope This idea sits outside of the TypeScript language design constraints labels Dec 14, 2016
@mhegazy mhegazy closed this as completed Apr 24, 2017
@alexeagle
Copy link
Contributor Author

note: closure compiler added --module_resolution=node which supports implicit import from /index so we don't need the workaround anymore.

@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Out of Scope This idea sits outside of the TypeScript language design constraints Suggestion An idea for TypeScript Too Complex An issue which adding support for may be too complex for the value it adds
Projects
None yet
Development

No branches or pull requests

5 participants