Babel very strange move comments

[burashka]

Babel very strange move comments: example, DojoDoc comments in function and info about copyright.

Input Code

// Available via the modified BSD license. See LICENSE for details.

import a from "a";

const ESC = 27;

var obj = {
  isValid: function(/*Boolean*/ /*===== isFocused =====*/) {
			return true;
		}
};

Babel Configuration (.babelrc, package.json, cli command)

es2015 preset

Expected Behavior

// Available via the modified BSD license. See LICENSE for details.
"use strict";

var _a = require("a");

var _a2 = _interopRequireDefault(_a);

function _interopRequireDefault(obj) { return obj && obj.__esModule ? obj : { default: obj }; }

var ESC = 27;

var obj = {
	isValid: function isValid(/*Boolean*/ /*===== isFocused =====*/){
		return true;
	}
};

Current Behavior

"use strict";

var _a = require("a");

var _a2 = _interopRequireDefault(_a);

function _interopRequireDefault(obj) { return obj && obj.__esModule ? obj : { default: obj }; }

var ESC = 27; // Available via the modified BSD license. See LICENSE for details.

var obj = {
	isValid: function isValid() /*Boolean*/ /*===== isFocused =====*/{
		return true;
	}
};

Your Environment

You can reproduce on your sandbox ("Try it out"): http://babeljs.io/repl/

[babel-bot]

Hey @burashka! We really appreciate you taking the time to report an issue. The collaborators
on this project attempt to help as many people as possible, but we're a limited number of volunteers,
so it's possible this won't be addressed swiftly.

If you need any help, or just have general Babel or JavaScript questions, we have a vibrant Slack
community that typically always has someone willing to help. You can sign-up here
for an invite.

[hzoo]

Thanks for the report, there's a few issues with comments already and probably all the same root cause of not knowing where to keep the comments https://github.com/babel/babel/issues?q=is%3Aissue+is%3Aopen+comments+label%3A%22comment+attachment%22

I think you just won't be able to rely on the fact that Babel will be able to put it make in the right place without us switching the way comments work.

The normal usecase for Babel is compiled/output code so whitespace/comments aren't our focus. This is why our printer doesn't have any options for things like quotes anymore.

[kaicataldo]

Comments are challenging because they can occur anywhere. The way the current algorithm works, comments are marked as "leading" and "trailing" comments on nodes. This method works in that all comments are accounted for (i.e. no comments will be lost), but isn't very precise as far being able to calculate where the comment should go after transformation.

This might be made better by thinking of comments in the context of token lists (i.e. the comment originally occurred after this opening parens token so let's put it back in right after the opening parens token), but I think there will always be cases where the comments will be in the "incorrect" location.

For example, for the following code, given the following comment locations:

function foo({ a = /* comment */ true }) {}
function foo({  /* comment */ a = true }) {}
function foo(/* comment */ {  a = true }) {}
function foo({  a = true /* comment */ }) {}
function foo({  a = true } /* comment */) {}

Where should the comments go in this transformed output for each respective comment location?

function foo(_ref) {
  var _ref$a = _ref.a,
      a = _ref$a === undefined ? true : _ref$a;
}

When the output is so different from the input, it's really hard to know, and I'm not sure it's worth solving this problem right now. Would you mind explaining the situations where the comments in the transpiled output are necessary? Would love to understand why you think this should be prioritized.

[burashka]

Thanks for your answer. We develops UI Framework for APS Standard. It based on Dojo Toolkit and use DojoDoc for auto-generate documentation.

DojoDoc have syntax: _attrToDom: function(/String/ attr, /String/ value){ or _attrToDom: function(/String/ /== attr ==/){.

We transforms ES modules with ES2015+ features to AMD modules and build it with help Dojo Builder. Dojo Builder try generate documentation and crashes on moved comments.

[dmail]

As #5503 demonstrates leading comments are messed up. I made PR #5504 to fix this issue, I haven't tested it on your example but I'm confident it would fix it.

But keep in mind that comment output remains strange. To see what I mean you can check tests associated to that PR. It will help you to understand where comments will be when a node is removed using path.remove().

[danielo515]

I have a similar issue with comments. My important comments are just headers. This means that I want my comments to stay on top of the code, and I want them to preserve the format.

In my case, funny things happens depending if the original code includes "use strict" or not.

For example, this code:

/*\
title: $:/plugins/danielo515/tiddlypouch/config/single-db-config
type: application/javascript
module-type: library


@preserve

\*/

function dbConfig(name, remote) {
  if ( typeof name === 'object' ){
      remote = name.remote;
      name = name.name;
    }
  this.name = name;
}

Produces an unexpected output that breaks my headers:

'use strict';

var _typeof = typeof Symbol === "function" && typeof Symbol.iterator === "symbol" ? function (obj) { return typeof obj; } : function (obj) { return obj && typeof Symbol === "function" && obj.constructor === Symbol && obj !== Symbol.prototype ? "symbol" : typeof obj; };

/*\
title: $:/plugins/danielo515/tiddlypouch/config/single-db-config
type: application/javascript
module-type: library


@preserve

\*/

function dbConfig(name, remote) {
  if ((typeof name === 'undefined' ? 'undefined' : _typeof(name)) === 'object') {
    remote = name.remote;
    name = name.name;
  }
  this.name = name;
}

BUT if I manually add use strict to the source code, then the expected output is generated:

input:

/*\
title: $:/plugins/danielo515/tiddlypouch/config/single-db-config
type: application/javascript
module-type: library


@preserve

\*/

'use strict'
function dbConfig(name, remote) {
  if ( typeof name === 'object' ){
      remote = name.remote;
      name = name.name;
    }
  this.name = name;
}

Output:

/*\
title: $:/plugins/danielo515/tiddlypouch/config/single-db-config
type: application/javascript
module-type: library


@preserve

\*/

'use strict';

var _typeof = typeof Symbol === "function" && typeof Symbol.iterator === "symbol" ? function (obj) { return typeof obj; } : function (obj) { return obj && typeof Symbol === "function" && obj.constructor === Symbol && obj !== Symbol.prototype ? "symbol" : typeof obj; };

function dbConfig(name, remote) {
  if ((typeof name === 'undefined' ? 'undefined' : _typeof(name)) === 'object') {
    remote = name.remote;
    name = name.name;
  }
  this.name = name;
}

Not sure if this is a feature or an unexpected behavior, but at least it allows me to save my comments, so please do not remove it. It will be also useful to document this.

Regards

[danielo515]

wort mention that if you place your use strict inside an IIFE then the first unexpected output comes back. For example, modifying the previous code like this:

/*\
title: $:/plugins/danielo515/tiddlypouch/config/single-db-config
type: application/javascript
module-type: library

@preserve

\*/

(function(){
'use strict'
function dbConfig(name, remote) {
  if ( typeof name === 'object' ){
      remote = name.remote;
      name = name.name;
    }
  this.name = name;
}})()

Will produce the previous unexpected output

[loganfsmyth]

@danielo515 The case you're referring to could just as easily go the other way. If I had

/**
 * @param {string} p1 Some function param
 */
function fn(p1){}

then

/**
 * @param {string} p1 Some function param
 */
"use strict";

function fn(p1){}

would be the opposite of what you'd expect because the docs are meant to be attached to the function, not the top of the file.

I don't see a way for us the tell the difference between these two cases.

[danielo515]

@loganfsmyth as you can see in my previous answer I found the way to workaround my problem. I was just specifying in which cases you get an output other than the desired. I'm fine about putting the use strict statement whenever makes sense to avoid this problem. I just wanted to make sure that this behavior is reported and also that it is not changed or removed.

Regards

[tilgovi]

Maybe @license is a good heuristic for keeping a comment at the top? That's a common pattern.

[tilgovi]

@babel/generator already handles @license specially.

[loganfsmyth]

The issue is that there is no "top" to keep things attached to. Comments just get attached to the thing they are nearest to.

It's not a great system for keeping comment locations, but handling comments with anything more than a best-effort attempt is not something Babel has ever tried to do. It would likely require that we use a full concrete syntax tree instead of a general abstract syntax tree, and it would make transforms likely much more complex.

To me it sounds like the best solution would be to update the documentation generator to run on the original source files.

[danielo515]

To me it sounds like the best solution would be to update the documentation generator to run on the original source files.

That makes sense in the case your objective is generating documentation. But some projects require comments and we just want to use the latest javascript with them.

Regards

[loganfsmyth]

some projects require comments

What kind of examples do you have in mind?

[tilgovi]

I'll give you my example, though I'm sure I can find a workaround.

It feels most correct to me to keep my license header in every source file. Since these headers have @license they cannot be stripped by --no-comments. I could add the license header with a plugin but I cannot strip it. I'm stuck having either no headers in the files of my repo and proper headers in my built package, or having headers in my repo and sometimes duplicate headers in my built package.

I understand there is no logical top of the file to attach a comment to, but there is an implicit top that exists by inserting a comment before the first expression, I would think. It certainly seems like I could write a plugin that strips @license headers from input files and re-inserts them at the end of compilation. I might try to write such a plugin, but I wonder if that's not a common enough use case that it deserves to be handled out of the box.

[loganfsmyth]

@tilgovi Here ya go: https://www.npmjs.com/package/babel-plugin-preserve-comment-header

[tilgovi]

@loganfsmyth that's incredibly nice of you. Since you know this stuff in and out I guess that might not have been huge effort for you, but I appreciate it deeply. Thanks for being amazing.

[loganfsmyth]

No problem! I was gonna post it in a comment and then decided I might as well just publish it :D

[danielo515]

Hello @loganfsmyth , tyddlywiki is an example. It uses magic headers on the JS files to determine the type of module, its name etc. What I'm doing is including the @preserve annotation (which works with some uglifiers). Maybe your new-born plugin can take it in account as default ?
Regards

[loganfsmyth]

@danielo515 That is likely something that module can do. It is configurable so you should be able to just set pattern: "@preserve" as an option for the plugin.