From 5e11ec14aa1b3d7ff31a40e6c22098d6fe63b3fb Mon Sep 17 00:00:00 2001
From: Chengzhong Wu <legendecas@gmail.com>
Date: Sat, 4 May 2024 14:55:06 +0100
Subject: [PATCH] build: define `NOMINMAX` in common.gypi
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

V8 and Node.js had defined `NOMINMAX` on Windows for a long time.  In
recent changes, V8 added `std::numeric_limits::min` usages in its
header files which caused addons without `NOMINMAX` defines failed
to compile.

Define `NOMINMAX` in common.gypi so that addons can be compiled with
the latest V8 header files.

PR-URL: https://github.com/nodejs/node/pull/52794
Fixes: https://github.com/nodejs/nan/issues/968
Refs: https://github.com/nodejs/gyp-next/pull/244
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz.nizipli@sentry.io>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
---
 common.gypi                         |  4 ++++
 node.gypi                           |  4 ----
 test/addons/hello-world/binding.gyp |  5 +++++
 test/addons/hello-world/binding2.cc | 21 +++++++++++++++++++++
 test/addons/hello-world/test.js     |  3 +++
 5 files changed, 33 insertions(+), 4 deletions(-)
 create mode 100644 test/addons/hello-world/binding2.cc

diff --git a/common.gypi b/common.gypi
index d6f024ab93a365..694f15ebd23c61 100644
--- a/common.gypi
+++ b/common.gypi
@@ -458,6 +458,10 @@
           '_HAS_EXCEPTIONS=0',
           'BUILDING_V8_SHARED=1',
           'BUILDING_UV_SHARED=1',
+          # Stop <windows.h> from defining macros that conflict with
+          # std::min() and std::max().  We don't use <windows.h> (much)
+          # but we still inherit it from uv.h.
+          'NOMINMAX',
         ],
       }],
       [ 'OS in "linux freebsd openbsd solaris aix os400"', {
diff --git a/node.gypi b/node.gypi
index 95133818dff7c7..e3c125fb9ec7f8 100644
--- a/node.gypi
+++ b/node.gypi
@@ -63,10 +63,6 @@
         'FD_SETSIZE=1024',
         # we need to use node's preferred "win32" rather than gyp's preferred "win"
         'NODE_PLATFORM="win32"',
-        # Stop <windows.h> from defining macros that conflict with
-        # std::min() and std::max().  We don't use <windows.h> (much)
-        # but we still inherit it from uv.h.
-        'NOMINMAX',
         '_UNICODE=1',
       ],
       'msvs_precompiled_header': 'tools/msvs/pch/node_pch.h',
diff --git a/test/addons/hello-world/binding.gyp b/test/addons/hello-world/binding.gyp
index 55fbe7050f18e4..a445af3d1cd18e 100644
--- a/test/addons/hello-world/binding.gyp
+++ b/test/addons/hello-world/binding.gyp
@@ -4,6 +4,11 @@
       'target_name': 'binding',
       'sources': [ 'binding.cc' ],
       'includes': ['../common.gypi'],
+    },
+    {
+      'target_name': 'binding2',
+      'sources': [ 'binding2.cc' ],
+      'includes': ['../common.gypi'],
     }
   ]
 }
diff --git a/test/addons/hello-world/binding2.cc b/test/addons/hello-world/binding2.cc
new file mode 100644
index 00000000000000..b94aecb3c76b26
--- /dev/null
+++ b/test/addons/hello-world/binding2.cc
@@ -0,0 +1,21 @@
+// Include uv.h and v8.h ahead of node.h to verify that node.h doesn't need to
+// be included first. Disable clang-format as it will sort the include lists.
+// clang-format off
+#include <uv.h>
+#include <v8.h>
+#include <node.h>
+// clang-format on
+
+static void Method(const v8::FunctionCallbackInfo<v8::Value>& args) {
+  v8::Isolate* isolate = args.GetIsolate();
+  args.GetReturnValue().Set(
+      v8::String::NewFromUtf8(isolate, "world").ToLocalChecked());
+}
+
+static void InitModule(v8::Local<v8::Object> exports,
+                       v8::Local<v8::Value> module,
+                       v8::Local<v8::Context> context) {
+  NODE_SET_METHOD(exports, "hello", Method);
+}
+
+NODE_MODULE(NODE_GYP_MODULE_NAME, InitModule)
diff --git a/test/addons/hello-world/test.js b/test/addons/hello-world/test.js
index d1d67d34f688c9..79f03f44be857a 100644
--- a/test/addons/hello-world/test.js
+++ b/test/addons/hello-world/test.js
@@ -6,6 +6,9 @@ const binding = require(bindingPath);
 assert.strictEqual(binding.hello(), 'world');
 console.log('binding.hello() =', binding.hello());
 
+const binding2 = require(require.resolve(`./build/${common.buildType}/binding2`));
+assert.strictEqual(binding2.hello(), 'world');
+
 // Test multiple loading of the same module.
 delete require.cache[bindingPath];
 const rebinding = require(bindingPath);