Adds a failing test for System JS compilation. #3376
Conversation
| execute: function() { | ||
| (function (Foo) { | ||
| })(Foo || (Foo = {})); | ||
| exports_1("Foo", Foo); |
There was a problem hiding this comment.
The compiler does not generate a semicolon at the end of this line, however it appears to be necessary. I am guessing that the JS runtime is seeing an attempt to call a function on exports_1("Foo", Foo)(...), since the following function is wrapped in parenthesis. The semicolon tells the runtime that a new expression is starting on line 19.
There was a problem hiding this comment.
You're right JS treats it as call statement. I think you can just add a semicolon to this line and you can submit a fix 👍
if (compilerOptions.module === ModuleKind.System && (node.flags & NodeFlags.Export)) {
// write the call to exporter for enum
writeLine();
write(`${exportFunctionForFile}("`);
emitDeclarationName(node);
write(`", `);
emitDeclarationName(node);
write(")"); // here
}https://github.com/Microsoft/TypeScript/blob/master/src/compiler/emitter.ts#L4506
|
Thanks @Zoltu, We should file an issue to track this. |
I do not know nearly enough about JavaScript or SystemJS to understand why the semicolon is necessary, but without it my application fails to load the module in chrome. A semicolon does not appear to be necessary on the second `exports_1(...)` call, though perhaps it would be easiest to just apply the semicolon more liberally in general to avoid dealing with JavaScript semicolon weirdness?
|
Looks like this is fixed in e34d28f. Feel free to accept the test as a regression test, or not if you believe the existing coverage is good enough. Thanks! |
|
i think the tests in place should catch the issue. thanks again for catching this! |
I do not know nearly enough about JavaScript or SystemJS to understand why the semicolon is necessary, but without it the module fails to load in chrome (unsure about other browsers). A semicolon does not appear to be necessary on the second
exports_1(...)call, though perhaps it would be easiest to just apply the semicolon more liberally in general to avoid dealing with JavaScript semicolon weirdness?Note: It is possible this test could be whittled down some more and the bug would still reproduce but I don't have an environment setup to get chrome to throw an error (outside of the application I am working on where I first saw the bug) so I didn't want to over-reduce it. I believe that fundamentally this is the issue I am seeing, hopefully a JavaScript expert can chime in and indicate what the minimum necessary repro case actually is.