Skip to content

Commit

Permalink
Don't loose comments when converting ESM to commonjs
Browse files Browse the repository at this point in the history
  • Loading branch information
nicolo-ribaudo committed Feb 15, 2023
1 parent fb76e44 commit f355531
Show file tree
Hide file tree
Showing 9 changed files with 73 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,8 @@ export interface SourceModuleMetadata {
loc: t.SourceLocation | undefined | null;
};
lazy?: Lazy;
leadingComments?: t.Comment[];
trailingComments?: t.Comment[];
}

export interface LocalExportMetadata {
Expand Down Expand Up @@ -245,7 +247,17 @@ function getModuleMetadata(
);

const sourceData = new Map();
const getData = (sourceNode: t.StringLiteral) => {
const getData = ({
source: sourceNode,
loc,
leadingComments,
trailingComments,
}: {
source: t.StringLiteral;
loc?: t.SourceLocation;
leadingComments?: t.Comment[];
trailingComments?: t.Comment[];
}) => {
const source = sourceNode.value;

let data = sourceData.get(source);
Expand All @@ -271,16 +283,24 @@ function getModuleMetadata(
lazy: false,

source,

leadingComments: null,
};
sourceData.set(source, data);
}
if (!data.loc) data.loc = loc;
if (leadingComments) {
(data.leadingComments ||= []).push(...leadingComments);
}
if (trailingComments) {
(data.trailingComments ||= []).push(...trailingComments);
}
return data;
};
let hasExports = false;
programPath.get("body").forEach(child => {
if (child.isImportDeclaration()) {
const data = getData(child.node.source);
if (!data.loc) data.loc = child.node.loc;
const data = getData(child.node);

child.get("specifiers").forEach(spec => {
if (spec.isImportDefaultSpecifier()) {
Expand Down Expand Up @@ -329,16 +349,16 @@ function getModuleMetadata(
});
} else if (child.isExportAllDeclaration()) {
hasExports = true;
const data = getData(child.node.source);
if (!data.loc) data.loc = child.node.loc;
const data = getData(child.node);

data.reexportAll = {
loc: child.node.loc,
};
} else if (child.isExportNamedDeclaration() && child.node.source) {
hasExports = true;
const data = getData(child.node.source);
if (!data.loc) data.loc = child.node.loc;
const data = getData(
child.node as t.ExportNamedDeclaration & { source: t.StringLiteral },
);

child.get("specifiers").forEach(spec => {
assertExportSpecifier(spec);
Expand Down
2 changes: 2 additions & 0 deletions packages/babel-plugin-transform-modules-commonjs/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,8 @@ export default declare((api, options: Options) => {
}
}
header.loc = metadata.loc;
header.leadingComments = metadata.leadingComments;
header.trailingComments = metadata.trailingComments;

headers.push(header);
headers.push(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@ Object.defineProperty(exports, "__esModule", {
});
var _exportNames = {};
exports.default = void 0;
// The fact that this exports both a normal default, and all of the names via
// re-export is an edge case that is important not to miss. See
// https://github.com/babel/babel/issues/8306 as an example.
var _react = babelHelpers.interopRequireWildcard(require("react"));
Object.keys(_react).forEach(function (key) {
if (key === "default" || key === "__esModule") return;
Expand All @@ -17,8 +20,5 @@ Object.keys(_react).forEach(function (key) {
}
});
});
// The fact that this exports both a normal default, and all of the names via
// re-export is an edge case that is important not to miss. See
// https://github.com/babel/babel/issues/8306 as an example.
var _default2 = _react.default;
exports.default = _default2;
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
/* istanbul ignore file */
import * as Something from 'some-dep';
// a comment
// export default function() {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"externalHelpers": true,
"plugins": ["transform-modules-commonjs"]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
"use strict";

/* istanbul ignore file */
var Something = babelHelpers.interopRequireWildcard(require("some-dep"));
// a comment
// export default function() {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
// a comment
import { useState } from "react";

// another comment
import "react-dom";

/* a third comment */
import * as R from "react";

// a class
class A extends useState(R) {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"externalHelpers": true,
"plugins": ["transform-modules-commonjs"]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
"use strict";

// a comment

/* a third comment */
var R = babelHelpers.interopRequireWildcard(require("react"));

// another comment

// a class
require("react-dom");
class A extends (0, R.useState)(R) {}

0 comments on commit f355531

Please sign in to comment.