Port to NAPI (#644)

* Port to NAPI The "5th pty bug" in #432 fixed also. * Fix help message in pty.cc * Move NAPI deps to devDependencies in package.json * Apply most of deepak1556's suggestions * Fix winpty * Fix conpty missing CloseHandle * Use unique_ptr to avoid `goto`s * Why macos failed? * fix: ci and minor cleanups * fix build failed on windows --------- Co-authored-by: deepak1556 <hop2deep@gmail.com>
microsoft · Jan 26, 2024 · b1fdda4 · b1fdda4
1 parent d6ce76a
commit b1fdda4
Show file tree

Hide file tree

Showing 10 changed files with 543 additions and 651 deletions.
diff --git a/azure-pipelines.yml b/azure-pipelines.yml
@@ -42,6 +42,9 @@ jobs:
     inputs:
       versionSpec: $(node_version)
     displayName: 'Install Node.js'
+  - script: |
+      python3 -m pip install setuptools
+    displayName: Install setuptools (macOS)
   - script: |
       npm i
     displayName: 'Install dependencies and build'
@@ -54,7 +57,7 @@ jobs:
 
 - job: Windows
   pool:
-    vmImage: 'windows-2019'
+    vmImage: 'windows-latest'
   strategy:
     matrix:
       node_16_x:
@@ -87,7 +90,7 @@ jobs:
   steps:
   - task: NodeTool@0
     inputs:
-      versionSpec: '16.x'
+      versionSpec: '18.x'
     displayName: 'Install Node.js'
   - script: |
       npm i

diff --git a/binding.gyp b/binding.gyp
@@ -1,5 +1,8 @@
 {
   'target_defaults': {
+    'dependencies': [
+      "<!(node -p \"require('node-addon-api').targets\"):node_addon_api_except",
+    ],
     'conditions': [
       ['OS=="win"', {
         'msvs_configuration_attributes': {
@@ -28,9 +31,6 @@
       'targets': [
         {
           'target_name': 'conpty',
-          'include_dirs' : [
-            '<!(node -e "require(\'nan\')")'
-          ],
           'sources' : [
             'src/win/conpty.cc',
             'src/win/path_util.cc'
@@ -41,17 +41,14 @@
         },
         {
           'target_name': 'conpty_console_list',
-          'include_dirs' : [
-            '<!(node -e "require(\'nan\')")'
-          ],
           'sources' : [
             'src/win/conpty_console_list.cc'
           ],
         },
         {
           'target_name': 'pty',
           'include_dirs' : [
-            '<!(node -e "require(\'nan\')")',
+            '<!(node -p "require(\'node-addon-api\').include_dir")',
             'deps/winpty/src/include',
           ],
           # Disabled due to winpty
@@ -73,9 +70,6 @@
       'targets': [
         {
           'target_name': 'pty',
-          'include_dirs' : [
-            '<!(node -e "require(\'nan\')")'
-          ],
           'sources': [
             'src/unix/pty.cc',
           ],
@@ -91,11 +85,6 @@
               'libraries!': [
                 '-lutil'
               ]
-            }],
-            ['OS=="mac"', {
-              "xcode_settings": {
-                "MACOSX_DEPLOYMENT_TARGET":"10.7"
-              }
             }]
           ]
         }

diff --git a/package.json b/package.json
@@ -43,7 +43,7 @@
     "prepublishOnly": "npm run build"
   },
   "dependencies": {
-    "nan": "^2.17.0"
+    "node-addon-api": "^7.1.0"
   },
   "devDependencies": {
     "@types/mocha": "^7.0.2",