Upgrade to Electron 43 - #22
Open
sandboxcoder wants to merge 2 commits into
Open
Conversation
There was a problem hiding this comment.
Pull request overview
Updates the build configuration to target Electron 43 headers and align CI tooling accordingly, ensuring native module builds use the correct runtime headers and Node.js toolchain.
Changes:
- Fixes macOS deployment target variable name so arm64 defaults can be applied.
- Bumps Electron header/runtime version to
v43.2.0in both CMake and CI. - Updates GitHub Actions macOS build to use Node.js
24.x.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| CMakeLists.txt | Fixes macOS deployment target variable typo and updates default runtime headers version to Electron v43.2.0. |
| ci/build-osx.sh | Adjusts how macOS deployment target is passed into CMake during CI builds. |
| .github/workflows/build.yml | Updates CI runtime headers version and Node.js version used for builds. |
Suppressed comments (1)
CMakeLists.txt:15
CMAKE_OSX_DEPLOYMENT_TARGETis set afterproject(). Now that the variable name typo is fixed, this block may still not reliably affect compiler/linker flags becauseCMAKE_OSX_DEPLOYMENT_TARGETis typically consumed during language/toolchain initialization (triggered byproject()/enable_language()). Consider setting it beforeproject()so the deployment target is applied consistently (especially important if CI stops passing-DCMAKE_OSX_DEPLOYMENT_TARGET).
cmake_minimum_required(VERSION 3.5)
project(node_window_rendering)
if(APPLE)
if (NOT CMAKE_OSX_ARCHITECTURES)
set(CMAKE_OSX_ARCHITECTURES "${CMAKE_HOST_SYSTEM_PROCESSOR}")
endif()
if (NOT CMAKE_OSX_DEPLOYMENT_TARGET)
if ("${CMAKE_OSX_ARCHITECTURES}" STREQUAL "arm64")
set(CMAKE_OSX_DEPLOYMENT_TARGET "11.0")
else()
set(CMAKE_OSX_DEPLOYMENT_TARGET "10.15")
endif()
endif()
endif()
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
34
to
38
| # Configure | ||
| cmake .. \ | ||
| -DCMAKE_OSX_DEPLOYMENT_TARGET=10.15 \ | ||
| -DCMAKE_OSX_DEPLOYMENT_TARGET=${CMAKE_OSX_DEPLOYMENT_TARGET} \ | ||
| -DCMAKE_BUILD_TYPE=RelWithDebInfo \ | ||
| -DCMAKE_INSTALL_PREFIX=${FULL_DISTRIBUTE_PATH}/node-window-rendering \ |
aleksandr-voitenko
approved these changes
Aug 25, 2026
summeroff
approved these changes
Aug 25, 2026
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
Suppressed comments (1)
ci/build-osx.sh:36
CMAKE_OSX_DEPLOYMENT_TARGETis not set anywhere in this script or the workflow env, so this expands to an empty-DCMAKE_OSX_DEPLOYMENT_TARGET=argument. That can populate the CMake cache with an empty deployment target and prevent the default logic inCMakeLists.txtfrom taking effect. Only pass this-Dflag when the variable is non-empty.
-DCMAKE_OSX_DEPLOYMENT_TARGET=${CMAKE_OSX_DEPLOYMENT_TARGET} \
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.