url: preserve property order in URLPattern results

Use each compiled component's group name list when materializing
URLPattern capture groups instead of iterating an unordered map.

Signed-off-by: Archkon <180910180+Archkon@users.noreply.github.com>

Author: Archkon

src/node_url_pattern.cc

@@ -409,22 +409,30 @@ std::optional<ada::url_pattern_init> URLPattern::URLPatternInit::FromJsObject(
 }
 
 MaybeLocal<Object> URLPattern::URLPatternComponentResult::ToJSObject(
-    Environment* env, const ada::url_pattern_component_result& result) {
+    Environment* env,
+    const ada::url_pattern_component_result& result,
+    const std::vector<std::string>& ordered_group_names) {
   auto isolate = env->isolate();
   auto context = env->context();
-  LocalVector<Name> group_names(isolate);
+  LocalVector<Name> js_group_names(isolate);
   LocalVector<Value> group_values(isolate);
-  group_names.reserve(result.groups.size());
+  js_group_names.reserve(result.groups.size());
   group_values.reserve(result.groups.size());
-  for (const auto& [group_key, group_value] : result.groups) {
+  // The result uses an unordered map, so follow the component's group name
+  // list to preserve capture declaration order.
+  for (const auto& group_key : ordered_group_names) {
+    const auto group = result.groups.find(group_key);
+    if (group == result.groups.end()) {
+      continue;
+    }
     Local<Value> key;
     if (!ToV8Value(context, group_key).ToLocal(&key)) {
       return {};
     }
-    group_names.push_back(key.As<Name>());
+    js_group_names.push_back(key.As<Name>());
     Local<Value> value;
-    if (group_value) {
-      if (!ToV8Value(env->context(), *group_value).ToLocal(&value)) {
+    if (group->second) {
+      if (!ToV8Value(env->context(), *group->second).ToLocal(&value)) {
         return {};
       }
     } else {
@@ -434,9 +442,9 @@ MaybeLocal<Object> URLPattern::URLPatternComponentResult::ToJSObject(
   }
   auto parsed_group = Object::New(isolate,
                                   Object::New(isolate),
-                                  group_names.data(),
+                                  js_group_names.data(),
                                   group_values.data(),
-                                  group_names.size());
+                                  js_group_names.size());
 
   Local<Value> input;
   if (!ToV8Value(env->context(), result.input).ToLocal(&input)) {
@@ -457,7 +465,9 @@ MaybeLocal<Object> URLPattern::URLPatternComponentResult::ToJSObject(
 }
 
 MaybeLocal<Value> URLPattern::URLPatternResult::ToJSValue(
-    Environment* env, const ada::url_pattern_result& result) {
+    Environment* env,
+    const ada::url_pattern_result& result,
+    const ada::url_pattern<URLPatternRegexProvider>& url_pattern) {
   auto isolate = env->isolate();
 
   auto tmpl = env->urlpatternresult_template();
@@ -479,8 +489,10 @@ MaybeLocal<Value> URLPattern::URLPatternResult::ToJSValue(
 
   size_t index = 0;
   MaybeLocal<Value> vals[] = {
-      URLPatternComponentResult::ToJSObject(env, result.hash),
-      URLPatternComponentResult::ToJSObject(env, result.hostname),
+      URLPatternComponentResult::ToJSObject(
+          env, result.hash, url_pattern.hash_component.group_name_list),
+      URLPatternComponentResult::ToJSObject(
+          env, result.hostname, url_pattern.hostname_component.group_name_list),
       Array::New(env->context(),
                  result.inputs.size(),
                  [&index, &inputs = result.inputs, env]() {
@@ -495,12 +507,20 @@ MaybeLocal<Value> URLPattern::URLPatternResult::ToJSValue(
                      return URLPatternInit::ToJsObject(env, init);
                    }
                  }),
-      URLPatternComponentResult::ToJSObject(env, result.password),
-      URLPatternComponentResult::ToJSObject(env, result.pathname),
-      URLPatternComponentResult::ToJSObject(env, result.port),
-      URLPatternComponentResult::ToJSObject(env, result.protocol),
-      URLPatternComponentResult::ToJSObject(env, result.search),
-      URLPatternComponentResult::ToJSObject(env, result.username)};
+      URLPatternComponentResult::ToJSObject(
+          env, result.password, url_pattern.password_component.group_name_list),
+      URLPatternComponentResult::ToJSObject(
+          env, result.pathname, url_pattern.pathname_component.group_name_list),
+      URLPatternComponentResult::ToJSObject(
+          env, result.port, url_pattern.port_component.group_name_list),
+      URLPatternComponentResult::ToJSObject(
+          env, result.protocol, url_pattern.protocol_component.group_name_list),
+      URLPatternComponentResult::ToJSObject(
+          env, result.search, url_pattern.search_component.group_name_list),
+      URLPatternComponentResult::ToJSObject(
+          env,
+          result.username,
+          url_pattern.username_component.group_name_list)};
   return NewDictionaryInstanceNullProto(env->context(), tmpl, vals);
 }
 
@@ -552,7 +572,7 @@ MaybeLocal<Value> URLPattern::Exec(Environment* env,
                                    std::optional<std::string_view>& baseURL) {
   if (auto result = url_pattern_.exec(input, baseURL ? &*baseURL : nullptr)) {
     if (result->has_value()) {
-      return URLPatternResult::ToJSValue(env, result->value());
+      return URLPatternResult::ToJSValue(env, result->value(), url_pattern_);
     }
     return Null(env->isolate());
   }

src/node_url_pattern.h

@@ -10,7 +10,9 @@
 #include <v8.h>
 
 #include <optional>
+#include <string>
 #include <string_view>
+#include <vector>
 
 namespace node::url_pattern {
 
@@ -81,13 +83,17 @@ class URLPattern : public BaseObject {
   class URLPatternResult {
    public:
     static v8::MaybeLocal<v8::Value> ToJSValue(
-        Environment* env, const ada::url_pattern_result& result);
+        Environment* env,
+        const ada::url_pattern_result& result,
+        const ada::url_pattern<URLPatternRegexProvider>& url_pattern);
   };
 
   class URLPatternComponentResult {
    public:
     static v8::MaybeLocal<v8::Object> ToJSObject(
-        Environment* env, const ada::url_pattern_component_result& result);
+        Environment* env,
+        const ada::url_pattern_component_result& result,
+        const std::vector<std::string>& ordered_group_names);
   };
 
  private:

test/parallel/test-urlpattern.js

@@ -50,3 +50,14 @@ assert.throws(() => {
   assert.strictEqual(result.pathname.input, '/test');
   assert.strictEqual(result.pathname.groups.value, 'test');
 }
+
+{
+  const result = new URLPattern({ pathname: '/:one/:two/:three' })
+    .exec('https://example.com/a/b/c');
+
+  assert.deepStrictEqual(Object.entries(result.pathname.groups), [
+    ['one', 'a'],
+    ['two', 'b'],
+    ['three', 'c'],
+  ]);
+}