From 3b56043612b1b161612b7ecc9448b45ae0949522 Mon Sep 17 00:00:00 2001
From: davidot <davidot@serenityos.org>
Date: Wed, 31 Aug 2022 20:31:02 +0200
Subject: [PATCH] LibJS: Put exports before symbols in keys of module namespace
 object

---
 .../LibJS/Runtime/ModuleNamespaceObject.cpp      | 11 +++++++----
 .../LibJS/Tests/modules/basic-modules.js         |  4 ++++
 .../LibJS/Tests/modules/namespace-order.mjs      | 16 ++++++++++++++++
 3 files changed, 27 insertions(+), 4 deletions(-)
 create mode 100644 Userland/Libraries/LibJS/Tests/modules/namespace-order.mjs

diff --git a/Userland/Libraries/LibJS/Runtime/ModuleNamespaceObject.cpp b/Userland/Libraries/LibJS/Runtime/ModuleNamespaceObject.cpp
index 2fda0b0e9c..2645a620c6 100644
--- a/Userland/Libraries/LibJS/Runtime/ModuleNamespaceObject.cpp
+++ b/Userland/Libraries/LibJS/Runtime/ModuleNamespaceObject.cpp
@@ -210,16 +210,19 @@ ThrowCompletionOr<bool> ModuleNamespaceObject::internal_delete(PropertyKey const
 ThrowCompletionOr<MarkedVector<Value>> ModuleNamespaceObject::internal_own_property_keys() const
 {
     // 1. Let exports be O.[[Exports]].
+    // NOTE: We only add the exports after we know the size of symbolKeys
+    MarkedVector<Value> exports { vm().heap() };
 
     // 2. Let symbolKeys be OrdinaryOwnPropertyKeys(O).
     auto symbol_keys = MUST(Object::internal_own_property_keys());
 
     // 3. Return the list-concatenation of exports and symbolKeys.
-    for (auto& export_name : m_exports) {
-        symbol_keys.append(js_string(vm(), export_name));
-    }
+    exports.ensure_capacity(m_exports.size() + symbol_keys.size());
+    for (auto const& export_name : m_exports)
+        exports.unchecked_append(js_string(vm(), export_name));
+    exports.extend(symbol_keys);
 
-    return symbol_keys;
+    return exports;
 }
 
 }
diff --git a/Userland/Libraries/LibJS/Tests/modules/basic-modules.js b/Userland/Libraries/LibJS/Tests/modules/basic-modules.js
index d0806c5e82..a9bd153b2c 100644
--- a/Userland/Libraries/LibJS/Tests/modules/basic-modules.js
+++ b/Userland/Libraries/LibJS/Tests/modules/basic-modules.js
@@ -182,6 +182,10 @@ describe("in- and exports", () => {
     test("can import with (useless) assertions", () => {
         expectModulePassed("./import-with-assertions.mjs");
     });
+
+    test("namespace has expected ordering", () => {
+        expectModulePassed("./namespace-order.mjs");
+    });
 });
 
 describe("loops", () => {
diff --git a/Userland/Libraries/LibJS/Tests/modules/namespace-order.mjs b/Userland/Libraries/LibJS/Tests/modules/namespace-order.mjs
new file mode 100644
index 0000000000..d9a0414429
--- /dev/null
+++ b/Userland/Libraries/LibJS/Tests/modules/namespace-order.mjs
@@ -0,0 +1,16 @@
+import * as ns from "./default-and-star-export.mjs";
+
+const keys = Reflect.ownKeys(ns);
+// The keys should be in alphabetical order and @@toString at the end
+if (keys.length < 4) throw new Error("Expected at least 3 keys and @@toStringTag");
+
+if (keys[0] !== "") throw new Error('Expected keys[0] === ""');
+
+if (keys[1] !== "*") throw new Error('Expected keys[1] === "*"');
+
+if (keys[2] !== "default") throw new Error('Expected keys[2] === "default"');
+
+if (keys.indexOf(Symbol.toStringTag) <= 2)
+    throw new Error("Expected Symbol.toStringTag to be behind string keys");
+
+export const passed = true;