comments before imports are stripped in AMD output unless there's a blank line after them

[dgoldstein0]

TypeScript Version: 3.0.1

Search Terms: preserve comments removes comments

Code
Compile the following with --module amd

// foo
import 'foo';

console.log('hi');

Expected behavior:
output contains // foo

Actual behavior:
output does not contain // foo.

If a newline is added between // foo and the import 'foo', the comment will then show up in the output. If the file is compiled with --module commonjs, the comment will also show up in the output, in that case regardless of the whitespace.

Playground Link: https://www.typescriptlang.org/play/#src=%2F%2F%20foo%0D%0Aimport%20'foo'%3B%0D%0A%0D%0Aconsole.log('hi')%3B%0D%0A

Related Issues: this looks most similar to #6399

[mhegazy]

Not sure where you expcext the coment to be.. the only thing i can think of is in the argument list, e.g.:

define(["require", "exports", /* foo */ "foo"], function (require, exports) {
    ...
});

Not sure if this is what you expect though..

[mhegazy]

To clarify, comments are considered "attached" to statements. when the statement is transformed, the comment is attached to the output. in CommonJs, the import statement is transformed into a require("foo"); statement, and the comment is maintained. In the AMD case, there is not clear place to add the comments, since the transformation was more drastic. Similarly if the whole import statement was elided the comment will not be generated in the output.

Comments at the beginning of a line followed by a new line are considered "unattached", and these are preserved, e.g. copyright headers.

[dgoldstein0]

Interesting. I expected that the comment would appear at the top of the output file approximately. For the use case I'm having trouble with, the comment is intended to say something about the whole file and really should not be considered attached to the import. Though as long as it appeared in the output somewhere, it would work for me.

…

On Tue, Jul 31, 2018, 10:03 AM Mohamed Hegazy ***@***.***> wrote: To clarify, comments are considered "attached" to statements. when the statement is transformed, the comment is attached to the output. in CommonJs, the import statement is transformed into a require("foo"); statement, and the comment is maintained. In the AMD case, there is not clear place to add the comments, since the transformation was more drastic. Similarly if the whole import statement was elided the comment will not be generated in the output. Comments at the beginning of a line followed by a new line are considered "unattached", and these are preserved, e.g. copyright headers. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#26079 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABBnd55630PdH7Lu9vy7SczNuuWk_0a7ks5uMI3zgaJpZM4Vnr3Q> .

[mhegazy]

For now i would recommend adding an empty new line after your comment.

[dgoldstein0]

I suppose I should give more context (though we've already did the extra newlines as a workaround).

I work at Dropbox on web platform. At the end of last year, I wrote a test that computes the set of js modules which are used, subtracts that from the set of all of our js modules, and fails if there are any dead modules founds. Since it's sometimes desirable to commit dead code (e.g. developing a new feature and writing some js for it first, before a later commit makes use of it), we added the ability to whitelist dead files by adding a comment like /* dead file: ... */ at the top, to explain why it's dead and what our plan for the code is.

The problem is that my test looks at the compiled js code for these comments - it was much easier to implement this way, than to figure out which compiled code corresponds to which sources and read the comments from the source. Occasionally, we'll get support requests of "why is this test still angry at me, I added the comment as it suggested"; while this isn't a very common question, we would like it to just work so that our engineers don't get held up on it. I filed this task here as an attempt to get the root cause fixed.

As for where I expect the comment to end up, I think I would have expected it to be above the define call, but either way works - we look for it with a regex so it doesn't matter what node it's next to.

[timocov]

The same happens for es2015 and commonjs modules too, but it seems that empty line does not help.

index.ts

// comment 1

import * as Module from './module';

// comment 2

import { SomeInterface } from './module';

export type A = typeof Module & SomeInterface;

module.ts

export interface SomeInterface {}

After compiling we get the next content in index.js (for es2015 module):

// comment 1

How to return comment 2 back in the output?

(moved to #1311)

[timocov]

Another usage case.

We use https://github.com/jsoverson/preprocess to remove some part of the code (depending of what kind of product we build right now) in the process of the compilation (until #3538 is fixed), and it is often necessary to remove imports (along with function or some part of the code).

Also the comment is removed if it is at the end of the expression, for example:

function foo() {
  return {
    foo: 5,
    // this comment will be emitted
    bar: 10,
    // but this one wouldn't
  };
}

(moved to #1311)

[dgoldstein0]

@timocov let's keep this task focused around comments in imports; in general it sounds like failure to preserve comments in the output is not a single bug but rather a class of bugs, so it would be much better to file such issues as separate tasks, and then just mention any bugs that look similar and let the maintainers merge bugs as they see fit.

In particular your second issue, about the comments in an object literal, should definitely be a separate bug.

Your first probably also deserves it's own bug - my quick read is that the comment is associated with a type only import, and Typescript omits that import in the output and such also omits the comment as a result. It seems like we'd want to debate whether this behavior is ideal or not, and a separate issue would probably make such discussion much easier to have.

[timocov]

@dgoldstein0 no problem. After some search I found it seems that more suitable issue for my cases - #1311. Sorry.

[dgoldstein0]

No need to apologize.

…

On Sun, Sep 2, 2018, 2:30 AM Evgeniy Timokhov ***@***.***> wrote: @dgoldstein0 <https://github.com/dgoldstein0> no problem. After some search I found it seems that more suitable issue for my cases - #1311 <#1311>. Sorry. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#26079 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABBndy2nIL7xwGmaYgsg9rsqg17oQj1Eks5uW6UugaJpZM4Vnr3Q> .