Fix moving comments of removed nodes

[nicolo-ribaudo]

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

[babel-bot]

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

[JLHwung]

IMO in this case the old behaviour is more preferable, /* before b */ is more likely to be the JSDoc comment of class property b.

[nicolo-ribaudo]

What do you think should be the priority? When we remove node B from ABC, we first try to attach B's trailing comments as leading comments of C, and then fallback to trailing comments of A?

[nicolo-ribaudo]

Ok actually, the priority I implemented is already like that.

What's happening here is that when we have this code:

class C {
  constructor() {}
  /* before b */
  static b;
  /* after b */
}

we call path.remove() on static b, and thus its comments are attached to constructor() {} which is the only available sibling node.

I'm confused by the previous output, because after b was attached to the constructor but before b wasn't 🤔

[nicolo-ribaudo]

I pushed a solution, I'm not sure I like it. The behavior I was confused about was because of the helpers for class fields to inherit comments that we recently introduced.

[ZeeshanTamboli]

I believe this fix is causing an issue for us at @mui. We transpile our TypeScript demos to formatted Javascript using babel. In the output JS files, the comments of the source Typescript files for the types also gets added.

TS file: (CustomColor.tsx)

// Update the Button's color prop options
declare module '@mui/material/Button' {
  interface ButtonPropsColorOverrides {
    neutral: true;
  }
}

export default function CustomColor() {
  return (
    <ThemeProvider theme={theme}>
      <Button color="neutral" variant="contained">
        neutral
      </Button>
    </ThemeProvider>
  );
}

Output JS file:

--- a/docs/data/material/customization/palette/CustomColor.js
+++ b/docs/data/material/customization/palette/CustomColor.js
@@ -11,6 +11,8 @@ const theme = createTheme({
   },
 });

+// Update the Button's color prop options
+
 export default function CustomColor() {
   return (
     <ThemeProvider theme={theme}>

This is a blocker for us to update babel to version 7.21.4. PR - mui/material-ui#36815

[mirabilos]

Is it possible that this is causing weird artefacts with top-level file-initial comments?

See danvk/dygraphs@a6fa54f...c814445#diff-065ff51ec7c9350ac6fae14523ffb237f5127eeeaffb2f80266c416c5df3b07eR6281 (click on the “Load diff” for the first file dist/dygraph.js, then scroll to the third hunk where a large amount of very badly indented comments is present)?

[mirabilos]

Yes, indeed, downgrading @babel/traverse to 7.21.3 fixes this issue for me.

I wouldn’t mind that much if the comments are retained as they are in the original file, but this utterly bad indentation and removing the line feeds between the two comments is totally inacceptable, and I’d rather not have the comments at all (as before) than that.

[JLHwung]

This is a blocker for us to update babel to version 7.21.4. PR - mui/material-ui#36815

@ZeeshanTamboli Can you open a new issue? So we can track the regression and see how we can fix it.

[ZeeshanTamboli]

This is a blocker for us to update babel to version 7.21.4. PR - mui/material-ui#36815

@ZeeshanTamboli Can you open a new issue? So we can track the regression and see how we can fix it.

@JLHwung Created issue #15579