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

Emit of require calls is invalid #9538

Closed
nullptr128 opened this issue Jul 6, 2016 · 12 comments
Closed

Emit of require calls is invalid #9538

nullptr128 opened this issue Jul 6, 2016 · 12 comments
Labels
Bug A bug in TypeScript Fixed A PR has been merged for this issue

Comments

@nullptr128
Copy link

TypeScript Version: nightly (2.0.0-dev.20160705)

Code

target: es6 , module: commonjs , moduleResolution: node
(tried target=es5 moduleResolution=classic as well)

File A (index.ts):

import * as TestModule from './test.ts';
let obj = new TestModule.Test();

File B (test.ts):
export class Test {}

Expected behavior:

Emitted index.js file should have:

const TestModule = require('./test.js');

Actual behavior:

Emited index.js file wrongly points at .ts file causing runtime module not found error.

const TestModule = require('./test.ts')

I've searched internet and looked towards BC changes in TypeScript but couldn find anything on this topic. I am 100% sure it changed extensions from .ts to .js in the earlier versions of TypeScript (around 1.5 or so).

@aluanhaddad
Copy link
Contributor

aluanhaddad commented Jul 6, 2016

@nullptr128 The behavior is 100% correct, and is expected.

You need to write:

File A (index.ts):

import * as TestModule from './test';
let obj = new TestModule.Test();

File B (test.ts):
export class Test {}

which will generate the correct code which is not

const TestModule = require('./test.js');

but rather

const TestModule = require('./test');

@mhegazy
Copy link
Contributor

mhegazy commented Jul 6, 2016

please see my replies in #9456

@mhegazy mhegazy closed this as completed Jul 6, 2016
@mhegazy mhegazy added the Question An issue which isn't directly actionable in code label Jul 6, 2016
@nullptr128
Copy link
Author

Ok thank you. Should TS compiler permit compiling a file referecing .ts file with extension permitted? Because I think it is weird behaviour that it compiles fine while it is non functional in runtime? I'd also propose to state this in documentation because it was quite hard to find out what is going on wrong here.

@mhegazy
Copy link
Contributor

mhegazy commented Jul 7, 2016

This is a good point. we have made changes this release that regressed the original behavior.

@mhegazy mhegazy added Bug A bug in TypeScript and removed Question An issue which isn't directly actionable in code labels Jul 7, 2016
@mhegazy mhegazy added this to the TypeScript 2.0.1 milestone Jul 7, 2016
@mhegazy mhegazy assigned ghost Jul 7, 2016
@mhegazy mhegazy reopened this Jul 7, 2016
@aluanhaddad
Copy link
Contributor

@mhegazy
In the linked issue you state

or if you are using TS 2.0:

import 'tests/interfaces/mocha.js';

which could be interpreted as meaning that one must (or should) use an explicit .js extension in TypeScript 2.0+. I don't believe that to be the case, but could you clarify your remarks?

@mhegazy
Copy link
Contributor

mhegazy commented Jul 7, 2016

Importing a module with no extension is allowed. Importing a module with a js extension is allowed too. Importing a module with a .ts should not.

@aluanhaddad
Copy link
Contributor

I understand and I appreciate the clarification, but I would like some guidance from the team as to which is preferable. I understand that it was introduced to help with SystemJS compatibility, and you know I'm all for that 😉 , but in actuality, the community doesn't really follow that. Generally default extension are configured with the loader via metadata, which results in a valid URL, but the actual modules are usually imported by a map or package name, especially in the case of typescript. If .js is part of a module name in the typescript can't be consumed directly.

@DanielRosenwasser
Copy link
Member

@aluanhaddad I think that this is in part contingent on how ES module loading behavior is defined for the browser. If your app requires browser compatibility, I'd personally consider using .js. Feel free to correct me if I'm wrong.

@aluanhaddad
Copy link
Contributor

aluanhaddad commented Jul 7, 2016

@DanielRosenwasser thank you for your response. I thought that the whatwg loader was going to be configurable with metadata to handle mapping.
So if someone were to import $ from 'jquery';, it would go through a mapping process to generate that URL in the context of the importing code, and that part of that metadata could be specified to append extensions, as it is in the SystemJS loader.

I don't see where this specified in the whatwg spec, and it may well have changed.

Personally I like the higher level of abstraction, not having to think about what language a module was authored in when depending on it, that omitting the extension brings. It also makes builds more flexible and integrating team workflows between developers easier, IMHO.

It seems, the ability to specify extensions in metadata will remain in the SystemJS loader polyfill (see systemjs/systemjs#1100 (comment) and systemjs/systemjs#1100 (comment)).

Obviously this does not imply the spec will ultimately provide for this.

@kitsonk
Copy link
Contributor

kitsonk commented Jul 7, 2016

I thought that the whatwg loader was going to be configurable with metadata to handle mapping.

I don't see where this specified in the whatwg spec, and it may well have changed.

It is far from baked as you are so very well pointing out. Until we actually have implementations in browsers that are considered feature complete, I don't think we can plan on anything fixed. It is good the Edge has at least given a partial implementation. At least it appears we have avoided the car crash of .mjs.

If your app requires browser compatibility, I'd personally consider using .js.

I generally disagree with this. The reality of loading resources in the browser will be visited upon those trying to implement module loading. Many lives have been lost upon the rocky shores of tightly coupling the module ID with a physical resource. The reasons for this are well articulated by James Burke in whatwg/loader#52 and there was discussion on some of the challenges here in #4595. Both transpiling, bundling and isomporphic code are all reasons to not tie your MID to a physical asset, which an extension implies. I am glad that TypeScript continues to be as fully compatible with JavaScript as possible, I am just disappointed in the wheel of re-invention that we as a community subject ourselves too.

@aluanhaddad
Copy link
Contributor

@kitsonk Thanks for pointing me to that discussion, it was very interesting to read through it. James Burke does make several important points. Specifically the concept of modules as references to abstract functionality is very relevant.

I think many people, myself included, conflate module names with their relative paths when importing (I leave off the extension for various reasons, but I am still thinking of where the module is when I import it), and this results in a somewhat limiting view. On the other hand, the notion that any arbitrary URL must always be able a potentially valid import has a number of benefits.

For better or worse the concept of modules in JavaScript seems to be ultimately tied to physical assets because the language and runtime lacks, or for a long time lacked, any notion of logical code organization. With physical code organization being a necessity which emerged quickly, logical code organization in JavaScript may have been, in certain communities, somewhat of an afterthought implemented as a layer on top of, and limited by, physical organization.

@kitsonk
Copy link
Contributor

kitsonk commented Jul 25, 2016

For better or worse the concept of modules in JavaScript seems to be ultimately tied to physical assets because the language and runtime lacks, or for a long time lacked, any notion of logical code organization. With physical code organization being a necessity which emerged quickly, logical code organization in JavaScript may have been, in certain communities, somewhat of an afterthought implemented as a layer on top of, and limited by, physical organization.

This was never the thought with AMD. It came out of James' frustration with Dojo modules (yes, Dojo has had modular JavaScript well before AMD, CJS, Node.js, SystemJS, WebPack, Browserify, etc. were a thing). AMD was specifically designed to work in a relative way and an absolute way without being constrained by direct access to a file system or making any assumptions about what modules are in what files. The where is a true logical construct which can be resolved to a default, configurable, mappable location.

@ghost ghost closed this as completed in #9646 Aug 19, 2016
@mhegazy mhegazy added the Fixed A PR has been merged for this issue label Aug 22, 2016
@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Bug A bug in TypeScript Fixed A PR has been merged for this issue
Projects
None yet
Development

No branches or pull requests

5 participants