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

allowJs: why is import "x.js" being ignored? #9070

Closed
OliverJAsh opened this issue Jun 10, 2016 · 18 comments
Closed

allowJs: why is import "x.js" being ignored? #9070

OliverJAsh opened this issue Jun 10, 2016 · 18 comments
Labels
Question An issue which isn't directly actionable in code

Comments

@OliverJAsh
Copy link
Contributor

Source:

// ./src/main.ts
import './console-log';
import './console-log.js';

console.log('console-log main')
// ./src/console-log.js
console.log('console-log JS')
// ./src/console-log.ts
console.log('console-log TS')

Config:

{
    "compilerOptions": {
        "module": "amd",
        "outFile": "./target/main.js",
        "allowJs": true
    },
    "files": ["./src/main.ts"]
}

Output:

// ./target/main.ts
console.log('console-log TS');
define("main", ["require", "exports", './console-log', './console-log.js'], function (require, exports) {
    "use strict";
    console.log('console-log main');
});

Notice that the output is missing the contents of the ./console-log.js import.

@kitsonk
Copy link
Contributor

kitsonk commented Jun 10, 2016

I suspect this is a dupe of #4595 and fixed via #8895

@OliverJAsh
Copy link
Contributor Author

Is there any workaround? I guess I could rename all of my files to .ts

@kitsonk
Copy link
Contributor

kitsonk commented Jun 10, 2016

I guess the question is why do you have to be explicit about the file extension on your modules?

I see you are outputting to AMD... it is considered not a good idea to specify the file extension in AMD, because of portability issues just like this.

@OliverJAsh
Copy link
Contributor Author

I have an app where some modules are TS, some are JS. I want to include all of them, but this bug means that currently, all my JS files are ignored.

I'm being explicit in my example because I have two files, one in TS and one in JS, and I want to import both. The fact they have the same name is irrelevant.

@kitsonk
Copy link
Contributor

kitsonk commented Jun 10, 2016

I'm being explicit in my example because I have two files, one in TS and one in JS, and I want to import both. The fact they have the same name is irrelevant.

Name two different modules .js and .ts is a recipe for disaster... When you files are transpiled and emitted, what do you think the .ts file is going to be named?

@OliverJAsh
Copy link
Contributor Author

The issue persists even if the files have different names.

@kitsonk
Copy link
Contributor

kitsonk commented Jun 10, 2016

But then you don't (and shouldn't) append .js to your file name for an AMD module loader.

@OliverJAsh
Copy link
Contributor Author

I see, that seems to fix it! Thank you.

On Fri, 10 Jun 2016 at 11:21 Kitson Kelly notifications@github.com wrote:

But then you don't (and shouldn't) append .js to your file name for an
AMD module loader.


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#9070 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/AA4QCa5zw196YN_9Tm7x4Xx-zJIHjquSks5qKTqOgaJpZM4Iyvi6
.

@OliverJAsh
Copy link
Contributor Author

OliverJAsh commented Jun 10, 2016

Now I have separate names, the modules are being included in the bundle. However, the AMD output is broken:

console.log('console-log JS');
console.log('console-log TS');
define("main", ["require", "exports", './console-log-1', './console-log-2'], function (require, exports) {
    "use strict";
    console.log('console-log main');
});

Requiring/importing main will fail because there are no AMD modules called console-log-1 or console-log-2. TypeScript did not wrap them as AMD modules. Why is this?

@DanielRosenwasser
Copy link
Member

DanielRosenwasser commented Jun 10, 2016

@OliverJAsh did console-log-1 or console-log-2 have any import or export statements? Our detection on whether or not a file is a module is based on that, and for global files, we just do simple concatentation. If you don't have any exports, you can use export {}.

@OliverJAsh
Copy link
Contributor Author

They have no import/export statements. Should TypeScript be adding them as dependencies to the outputted module if they are not detected to be a module?

In my case I am gradually converting an app into modules, so it would be difficult to add export everywhere.

@mhegazy
Copy link
Contributor

mhegazy commented Jun 10, 2016

there are two issues, first the .js extension issue. this is tracked by #4595 and fixed in #8895 as @kitsonk noted.

the second is that the files are not considered modules, as @DanielRosenwasser noted. for these, consider adding an empty export statement in the JS files, e.g.:

// ./src/console-log.js
console.log('console-log JS')

export {}; // makes it a module

with that change, the output would look like:

define("console-log", ["require", "exports"], function (require, exports) {
    "use strict";
    // ./src/console-log.ts
    console.log('console-log TS');
});
define("console-log", ["require", "exports"], function (require, exports) {
    "use strict";
    // ./src/console-log.js
    console.log('console-log JS');
});
define("main", ["require", "exports", "console-log", "console-log"], function (require, exports) {
    "use strict";
    console.log('console-log main');
});

The change that needs to be done on the TS compiler side, is to treat a file as module, if it was imported. this is more inline with the spec. we should file a different issue to track that.

@mhegazy mhegazy added the Question An issue which isn't directly actionable in code label Jun 10, 2016
@OliverJAsh
Copy link
Contributor Author

I think TypeScript should not allow the user to end up in the situation I did, with a broken AMD module. Could TypeScript throw an error/warn if you try to load a JS file that is not a module?

@kitsonk
Copy link
Contributor

kitsonk commented Jun 12, 2016

@OliverJAsh, covered by #9082

@mjohnsonengr
Copy link

I have a similar issue I posted in a StackOverflow question and then I stumbled across this -- it seems related, except I think my question points out that @mhegazy's suggestion actually does not produce the expected output posted.

@mhegazy
Copy link
Contributor

mhegazy commented Jun 13, 2016

@mjohnsonengr is this a different issue. i have replied to your SO question.

@mhegazy mhegazy closed this as completed Jun 13, 2016
@mhegazy
Copy link
Contributor

mhegazy commented Jun 13, 2016

Error for non-module code used as module is tracked by #9082

@aluanhaddad
Copy link
Contributor

@OliverJAsh The problems with explicit extension syntax are, as @kitsonk mentioned, name collision and loader issues, and also workflow toolchain issues.
Let's suppose I am developing a module using SystemJS and using plugin-typescript as my transpiler. My colleague is developing a different module and prefers to use gulp-typescript. I can consume his code and he can consume mine without any issues. However, if either of use used explicit extensions, they would be emitted in the transpiled code, meaning if my colleague were to import one of my files, it would be looking for the artifact. It also would mean that with allowJS the compiler would pick a .js file before a corresponding .ts file, and thereby lose available type information.

@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
Question An issue which isn't directly actionable in code
Projects
None yet
Development

No branches or pull requests

6 participants