Fix double-loading packages on npm (#132)

CHANGELOG.md

@@ -1,3 +1,8 @@
+## 2.4.7
+g
+* Fix a bug where npm packages could crash on Node.js if loaded both through
+  `require()` and `import`.
+
 ## 2.4.6
 
 * Properly mark NPM packages as `"type": "module"` when `pkg.jsEsmExports` is

lib/src/npm.dart

@@ -579,7 +579,11 @@ void _writePlatformWrapper(String path, JSRequireSet requires,
     {bool node = false}) {
   var exports = jsEsmExports.value;
   if (exports != null) {
-    _writeImportWrapper('$path.${node ? 'mjs' : 'js'}', requires, exports);
+    if (node) {
+      _writeNodeImportWrapper('$path.mjs', exports);
+    } else {
+      _writeImportWrapper('$path.${node ? 'mjs' : 'js'}', requires, exports);
+    }
     _writeRequireWrapper('$path.${node ? 'js' : 'cjs'}', requires);
   } else {
     _writeRequireWrapper('$path.js', requires);
@@ -613,6 +617,25 @@ String _loadRequires(JSRequireSet requires) {
   return buffer.toString();
 }
 
+/// Writes a wrapper to [path] that loads and re-exports `$_npmName.node.js`
+/// using ESM imports.
+///
+/// Rather than having a totally separate ESM wrapper, for Node we load ESM
+/// exports *through* the require wrapper. This ensures that we don't run into
+/// issues like sass/dart-sass#2017 if both are loaded in the same Node process.
+///
+/// [exports] is the value of [jsEsmExports].
+void _writeNodeImportWrapper(String path, Set<String> exports) {
+  var cjsUrl = './' + p.setExtension(p.basename(path), '.js');
+  var buffer = StringBuffer("import cjs from ${json.encode(cjsUrl)};\n\n");
+
+  for (var export in exports) {
+    buffer.writeln("export const $export = cjs.$export;");
+  }
+
+  writeString(path, buffer.toString());
+}
+
 /// Writes a wrapper to [path] that loads and re-exports `$_npmName.dart.js`
 /// using ESM imports with [requires] injected.
 ///

pubspec.yaml

@@ -1,5 +1,5 @@
 name: cli_pkg
-version: 2.4.6
+version: 2.4.7
 description: Grinder tasks for releasing Dart CLI packages.
 homepage: https://github.com/google/dart_cli_pkg
 

test/npm_test.dart

@@ -701,6 +701,11 @@ void main() {
           const myApp = require("my_app");
 
           console.log(myApp.hello);
+        """),
+        // Regression test for sass/dart-sass#2017
+        d.file("both.mjs", """
+          import "./test.mjs";
+          import "./test.cjs";
         """)
       ]).create();
 
@@ -717,6 +722,12 @@ void main() {
           await TestProcess.start("node$dotExe", [d.path("depender/test.cjs")]);
       expect(cjsProcess.stdout, emits("true"));
       await cjsProcess.shouldExit(0);
+
+      var bothProcess =
+          await TestProcess.start("node$dotExe", [d.path("depender/both.mjs")]);
+      expect(bothProcess.stdout, emits("true"));
+      expect(bothProcess.stdout, emits("true"));
+      await bothProcess.shouldExit(0);
     });
 
     test("overwrite existing string value", () async {