Skip to content

Conversation

@Robinsstudio
Copy link

@Robinsstudio Robinsstudio commented Dec 24, 2020

Q                       A
Fixed Issues? Fixes #11971
Patch: Bug Fix? Yes
Major: Breaking Change? No
Minor: New Feature? No
Tests Added + Pass? Yes
Documentation PR Link
Any Dependency Changes? No
License MIT

As explained in #11971, the first comment is sometimes removed while it shouldn't. Consider the following snippet from the issue:

// test
import React from 'react';

export default class A extends B {}

The bug occurs during the transformation of the AST. When the ImportDeclaration node is removed, its leading comment should be transferred to one of its new sibling nodes. And it pretty much does unless you want to compile for IE 11 or lower which causes some extra JavaScript code to be generated in the top of the file.

This results in a failure in two if statements. The comment will be added to the previous node if the removed node doesn't have a next node or to the next node if it doesn't have a previous node. If it has both a previous and a next node, then the comment is simply discarded.

@babel-bot
Copy link
Collaborator

Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/36471/

@codesandbox-ci
Copy link

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 06153ec:

Sandbox Source
babel-repl-custom-plugin Configuration
babel-plugin-multi-config Configuration
babel-repl-custom-plugin Issue #11971

@nicolo-ribaudo nicolo-ribaudo added area: comments PR: Bug Fix 🐛 A type of pull request used for our changelog categories labels Jan 6, 2021
Comment on lines 23 to 27
if (hasPrev && !hasNext) {
prev.addComments("trailing", trailing);
} else if (hasNext && !hasPrev) {
} else if (hasNext) {
next.addComments("leading", leading);
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the logic we had is wrong, and we should do something like this to avoid loosing comments?

if (hasPrev) {
  prev.addComments("trailing", leading);
} else if (hasNext) {
  next.addComments("leading", leading);
}
if (hasNext) {
  next.addComments("leading", trailing);
} else if (hasPrev) {
  prev.addComments("trailing", trailing);
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: comments PR: Bug Fix 🐛 A type of pull request used for our changelog categories

Projects

None yet

Development

Successfully merging this pull request may close these issues.

The first-line comment is been removed unexpectedly

3 participants