Repository: swc-project/swc (rust)

Issue:
Runtime issue with named export overrides


**Describe the bug**
Based on https://github.com/swc-project/swc/issues/1714

Valid JS is generated, but at runtime if the override is not the same (identity-wise) as the export from `*` the code won't load.

**Input code**
```
import { customRender } from './customRender';

// re-export everything
export * from '@testing-library/react';

// override render method
export { customRender as render };
```

**Config**

```json
{
  "sourceMaps": true,
  "jsc": {
    "parser": {
      "syntax": "typescript",
      "tsx": true,
      "dynamicImport": true
    },
    "transform": {
      "react": {
        "runtime": "automatic",
        "refresh": true
      },
      "hidden": {
        "jest": true
      }
    }
  },
  "module": {
    "type": "commonjs"
  },
  "env": {
    "coreJs": 3,
    "mode": "usage"
  }
}
```

**Expected behavior**
The transpilation should respect the behaviour that the named exports can override the wildcard exports.
e.g. [Babel REPL](https://babeljs.io/repl#?browsers=node%2014&build=&builtIns=false&corejs=3.6&spec=false&loose=false&code_lz=JYWwDg9gTgLgBAbzgYwK4GcYRAJQKYB2AJnlHAL5wBmU2cA5AHQD0am2-xp9A3AFB9mzOFDwBaPAA9IsOHgBupAJ4wAFsAIBzPlJnwAVNVogGAARh5MGzWIA2wAEZQAhlCXNRz5DF4ChcCEUoKGASEUISMhA8NQgiHWloeCQ2LFwI0jhndHCuMnJ-IA&debug=false&forceAllTransforms=false&shippedProposals=false&circleciRepo=&evaluate=false&fileSize=false&timeTravel=false&sourceType=module&lineWrap=true&presets=env,react&prettier=false&targets=Node-14&version=7.14.1&externalPlugins=) generates the `_exportNames` object for overrides and then excludes those when defining the wildcard export.
```
var _exportNames = {
  render: true
};

Object.keys(_react).forEach(function (key) {
  if (key === "default" || key === "__esModule") return;
  if (Object.prototype.hasOwnProperty.call(_exportNames, key)) return; // <-- This check is missing in the swc output
  if (key in exports && exports[key] === _react[key]) return; // <-- previous fix added this check, but only works when the override is the same as the * export
  Object.defineProperty(exports, key, {
    enumerable: true,
    get: function () {
      return _react[key];
    }
  });
});
```

**Version**
The version of @swc/core: 1.2.77

**Additional context**
This code should have this behaviour too:
```
export { customRender as render } from './customRender';

// re-export everything, but keep named 'render' exported above
export * from '@testing-library/react';
```


Pull request description:
fix(es/transforms/module/commonjs): fix named exports overrides
This PR closes #2101 and the details can be found in the issue description.

This is somewhat of a followup to PR #2583, as the PR was a incomplete solution.  It only fixed named exports declared as import/export statements.  Furthermore, the order of the named exports could appear before or after the wildcard exports, causing cases to be missed.

This fix requires a preliminary pass through the items to find all named exports first in order to address the ordering.  Furthermore, I've moved the logic from PR #2583 into this preliminary loop instead to catch both cases where export/imports are declared as well as simple named exports.

I've used Babel REPL to compare my output to multiple use cases, which also required changing a couple of internal test outputs.  

This re-ordering can cause the `_exportNames` array to be declared sooner than previously found.  I've found that Babel seems to always declare it right away.

It should also be noted that the testing has changed much since my first PR, and I've tried to follow the same patterns and locations to place and run tests.  

Patch:
diff --git a/crates/swc_ecma_transforms_module/src/common_js.rs b/crates/swc_ecma_transforms_module/src/common_js.rs
index 5375892cc2..b48d480fd4 100644
--- a/crates/swc_ecma_transforms_module/src/common_js.rs
+++ b/crates/swc_ecma_transforms_module/src/common_js.rs
@@ -89,6 +89,31 @@ where
         // Used only if export * exists
         let mut exported_names: Option<Ident> = None;
 
+        // make a preliminary pass through to collect exported names ahead of time
+        for item in &items {
+            match item {
+                ModuleItem::ModuleDecl(ModuleDecl::ExportNamed(NamedExport {
+                    ref specifiers,
+                    ..
+                })) => {
+                    for ExportNamedSpecifier { orig, exported, .. } in
+                        specifiers.into_iter().filter_map(|e| match e {
+                            ExportSpecifier::Named(e) => Some(e),
+                            _ => None,
+                        })
+                    {
+                        if let Some(exported) = &exported {
+                            exports.push(exported.sym.clone());
+                        } else {
+                            exports.push(orig.sym.clone());
+                        }
+                    }
+                }
+
+                _ => {}
+            }
+        }
+
         for item in items {
             self.in_top_level = true;
 
@@ -158,21 +183,9 @@ where
                             ref specifiers,
                             ..
                         })) => {
-                            // handle: export {sym as alias1, alias2, ...} from "x"
-                            for ExportNamedSpecifier { orig, exported, .. } in
-                                specifiers.into_iter().filter_map(|e| match e {
-                                    ExportSpecifier::Named(e) => Some(e),
-                                    _ => None,
-                                })
-                            {
-                                if let Some(exported) = &exported {
-                                    exports.push(exported.sym.clone());
-                                } else {
-                                    exports.push(orig.sym.clone());
-                                }
-                            }
                             scope.import_to_export(&src, !specifiers.is_empty());
                         }
+
                         _ => {}
                     }
                     drop(scope);


Test patch:
diff --git a/crates/swc/tests/fixture/issue-1714/case1/output/index.js b/crates/swc/tests/fixture/issue-1714/case1/output/index.js
index b6625cc9bd..26fb3e02c4 100644
--- a/crates/swc/tests/fixture/issue-1714/case1/output/index.js
+++ b/crates/swc/tests/fixture/issue-1714/case1/output/index.js
@@ -2,6 +2,9 @@
 Object.defineProperty(exports, "__esModule", {
     value: true
 });
+var _exportNames = {
+    render: true
+};
 Object.defineProperty(exports, "render", {
     enumerable: true,
     get: function() {
@@ -12,6 +15,7 @@ var _customRender = require("./customRender");
 var _react = _interopRequireWildcard(require("@testing-library/react"));
 Object.keys(_react).forEach(function(key) {
     if (key === "default" || key === "__esModule") return;
+    if (Object.prototype.hasOwnProperty.call(_exportNames, key)) return;
     if (key in exports && exports[key] === _react[key]) return;
     Object.defineProperty(exports, key, {
         enumerable: true,
diff --git a/crates/swc_ecma_transforms_module/tests/common_js.rs b/crates/swc_ecma_transforms_module/tests/common_js.rs
index 5949b4ecaa..c4c456e3ff 100644
--- a/crates/swc_ecma_transforms_module/tests/common_js.rs
+++ b/crates/swc_ecma_transforms_module/tests/common_js.rs
@@ -4161,6 +4161,9 @@ test!(
 Object.defineProperty(exports, "__esModule", {
     value: true
 });
+var _exportNames = {
+    Scope: true
+};
 Object.defineProperty(exports, "Scope", {
   enumerable: true,
   get: function() {
@@ -4170,6 +4173,7 @@ Object.defineProperty(exports, "Scope", {
 var _http = require("./http");
 Object.keys(_http).forEach(function(key) {
     if (key === "default" || key === "__esModule") return;
+    if (Object.prototype.hasOwnProperty.call(_exportNames, key)) return;
     if (key in exports && exports[key] === _http[key]) return;
     Object.defineProperty(exports, key, {
         enumerable: true,
@@ -4200,15 +4204,15 @@ export * from './pipes';
     Object.defineProperty(exports, "__esModule", {
         value: true
     });
+    var _exportNames = {
+      id: true
+    };
     Object.defineProperty(exports, "id", {
         enumerable: true,
         get: function() {
             return _interfaces.id;
         }
     });
-    var _exportNames = {
-        id: true
-    };
     require("reflect-metadata");
     var _http = require("./http");
     Object.keys(_http).forEach(function(key) {
diff --git a/crates/swc_ecma_transforms_module/tests/fixture/commonjs/issue-2101/1/input.js b/crates/swc_ecma_transforms_module/tests/fixture/commonjs/issue-2101/1/input.js
new file mode 100644
index 0000000000..d9a316de4b
--- /dev/null
+++ b/crates/swc_ecma_transforms_module/tests/fixture/commonjs/issue-2101/1/input.js
@@ -0,0 +1,7 @@
+import { customRender } from './customRender';
+
+// re-export everything
+export * from '@testing-library/react';
+
+// override render method
+export { customRender as render };
diff --git a/crates/swc_ecma_transforms_module/tests/fixture/commonjs/issue-2101/1/output.js b/crates/swc_ecma_transforms_module/tests/fixture/commonjs/issue-2101/1/output.js
new file mode 100644
index 0000000000..3a8e1443e2
--- /dev/null
+++ b/crates/swc_ecma_transforms_module/tests/fixture/commonjs/issue-2101/1/output.js
@@ -0,0 +1,26 @@
+"use strict";
+Object.defineProperty(exports, "__esModule", {
+    value: true
+});
+var _exportNames = {
+  render: true
+};
+Object.defineProperty(exports, "render", {
+    enumerable: true,
+    get: function() {
+        return _customRender.customRender;
+    }
+});
+var _customRender = require("./customRender");
+var _react = require("@testing-library/react");
+Object.keys(_react).forEach(function(key) {
+    if (key === "default" || key === "__esModule") return;
+    if (Object.prototype.hasOwnProperty.call(_exportNames, key)) return;
+    if (key in exports && exports[key] === _react[key]) return;
+    Object.defineProperty(exports, key, {
+        enumerable: true,
+        get: function() {
+            return _react[key];
+        }
+    });
+});
diff --git a/crates/swc_ecma_transforms_module/tests/fixture/commonjs/issue-2101/2/input.js b/crates/swc_ecma_transforms_module/tests/fixture/commonjs/issue-2101/2/input.js
new file mode 100644
index 0000000000..a13caef5c3
--- /dev/null
+++ b/crates/swc_ecma_transforms_module/tests/fixture/commonjs/issue-2101/2/input.js
@@ -0,0 +1,9 @@
+import { customRender } from './customRender';
+
+// override render method
+export { customRender as render };
+
+// re-ordering the export wildcard from case 1 should make no difference to output
+
+// re-export everything
+export * from '@testing-library/react';
diff --git a/crates/swc_ecma_transforms_module/tests/fixture/commonjs/issue-2101/2/output.js b/crates/swc_ecma_transforms_module/tests/fixture/commonjs/issue-2101/2/output.js
new file mode 100644
index 0000000000..f9f5c89040
--- /dev/null
+++ b/crates/swc_ecma_transforms_module/tests/fixture/commonjs/issue-2101/2/output.js
@@ -0,0 +1,26 @@
+"use strict";
+Object.defineProperty(exports, "__esModule", {
+    value: true
+});
+Object.defineProperty(exports, "render", {
+    enumerable: true,
+    get: function() {
+        return _customRender.customRender;
+    }
+});
+var _exportNames = {
+    render: true
+  };
+var _customRender = require("./customRender");
+var _react = require("@testing-library/react");
+Object.keys(_react).forEach(function(key) {
+    if (key === "default" || key === "__esModule") return;
+    if (Object.prototype.hasOwnProperty.call(_exportNames, key)) return;
+    if (key in exports && exports[key] === _react[key]) return;
+    Object.defineProperty(exports, key, {
+        enumerable: true,
+        get: function() {
+            return _react[key];
+        }
+    });
+});