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

Standardize "import {..} from 'path'" and make it not be specific to the way you compile #6409

Closed
corneliutusnea opened this issue Jan 9, 2016 · 6 comments
Labels
Declined The issue was declined as something which matches the TypeScript vision Suggestion An idea for TypeScript

Comments

@corneliutusnea
Copy link

Project structure

[root]
    /index.ts
    /abc/
       a.ts
    /other/
        o.ts
        /abc/
            a.ts 

Currently the statement import {a} from 'abc/a" from o.ts will import a different a class depending on how this is compiled.
If you compile as systemjs the imported 'a' is from file [root]/abc/a.
If you compile as commonjs the imported 'a' is from file ./abc/a so file [root]/other/abc/a
This makes coding and sharing .ts files a pain as there is no guarantee for where the files should be loaded from.

Proposal

import statements in .ts files should be standardized to always follow a single well defined convention that is independent on the way the files are compiled.
Just pick for example the systemjs convention and say:
imports are always relative to the declared [root] of the project

Then based on how you compile in systemjs/amd/commonjs ... the paths will be correctly adjusted.

For SystemJS I'd expect the file o.js to look like

System.register(['abc/a'], function(exports_1)

Which would load [root]/abc/a.js

For CommonJS I'd expect the generated o.js to have the paths adjusted:

var a_1 = require('../abc/a');

Which would also load [root]/abc/a.js

Right now this commonjs compilation generates for o.js

var a_1 = require('abc/a');

which loads '[root]/abc/other/abc/a'

This is somehow related to #2338 and #5039 however I both those seems to approach the issue from the loader point of view.

As a compiler I expect the inputs to the compiler (.ts files/.d.ts) to be standardized and follow a strict convention while the compiler's job is to correctly adjust the output to the various loaders or environments.

@corneliutusnea
Copy link
Author

On Gitter there was a recommendation to use --moduleResolution [node|classic].
The only thing that the option seems to do is fix the import during the compilation and change the way the paths are resolved. It does not correctly adjust the exported paths to respect the different loaders.

@mhegazy
Copy link
Contributor

mhegazy commented Jan 12, 2016

I do not think this is something the compiler should do. the compiler will not touch your module names, nor should it, as module names are not strictly file names, e.g. consider requireJS plugins. What the compiler is trying to do is to to infer from your imports the location of the module at design time.

@mhegazy mhegazy added Suggestion An idea for TypeScript Declined The issue was declined as something which matches the TypeScript vision labels Jan 12, 2016
@corneliutusnea
Copy link
Author

@mhegazy The compiler is already standardizing everything in TS. Classes, modules, properties .. that are not really part of JS. Why not also the import statements?
In Java, Ruby, C# ... the import/using statements don't have to be changed/adjusted based on how you compile or the output.
The role of a compiler is to have a standard input that compiles an expected output based on a set of flags.
Currently it's really impossible to have a shared library of .ts files that gets compiled for system.js for the browser and commonjs for node simply because tsc relies on it's expected output to resolve the input.

I understand the usage of imports when using external modules (requirejs or tsds) that in that scenario I agree that the compiler should keep the path but when importing OTHER .ts files the specification of the import should be clearly defined and not based on some output compilation flag.

@basarat
Copy link
Contributor

basarat commented Jan 18, 2016

My 2c. I always felt that changing the moduleResolution automatically based module was a bad idea. In fact in my dev tools I always assume moduleResolution : node unless the user explicitly changes it. This is also because of me being lazy and letting the user be explicit about what they want without me having to maintain this fact in my code base 🌹

@corneliutusnea
Copy link
Author

So @RyanCavanaugh that means TS will NOT be able correctly cross-compile modules between NodeJS and SystemJS?

@mhegazy
Copy link
Contributor

mhegazy commented Jan 20, 2016

I do not think this is a correct statement. You can use relative path, and would grantee that you are using the same module in both systems.

Module loaders have diffrences about what a package is, and where the root is. i do not think the TS compiler should hide all diffrences from users. importing non-relative package names mean something diffrent from one module system to another, and is impacted by configuration and path mapping logic that makes sense in some but not all loaders.

@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
Declined The issue was declined as something which matches the TypeScript vision Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests

4 participants