Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat!: bump engines requirement to Node 22 #1167

Open
wants to merge 16 commits into
base: main
Choose a base branch
from
Open

feat!: bump engines requirement to Node 22 #1167

wants to merge 16 commits into from

Conversation

erikian
Copy link
Member

@erikian erikian commented Jan 16, 2025

BREAKING CHANGE: bumps required Node.js version to >=22.12.0. ESM-only.

I also took the opportunity to update farmhash to 3.3.1 in one of the test fixtures so we can run a couple of test cases we were skipping on arm64 macOS. I tried switching to 4.0.1 but it would require some deeper changes in the tests, so I figured it would be a good idea to address that in a follow-up PR as 3.3.1 is enough to get those tests running for the Node upgrade.

@electron electron deleted a comment from notion-workspace bot Jan 18, 2025
@erikian erikian marked this pull request as ready for review February 10, 2025 02:58
@erikian erikian requested a review from a team as a code owner February 10, 2025 02:59
Copy link
Member

@felixrieseberg felixrieseberg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks clean and good!

@@ -0,0 +1,2 @@
*.d.ts
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting! I'm not against it, just surprised that eslint can't handle d.ts files.

@@ -102,7 +102,7 @@ for Electron Packager. For example:

```javascript
import packager from '@electron/packager';
import rebuild from '@electron/rebuild';
import { rebuild } from '@electron/rebuild';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👏

@@ -155,7 +155,7 @@ const childProcess = require('child_process');
const pathToElectron = require('electron');

rebuild({
buildPath: __dirname,
buildPath: import.meta.dirname,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we should document both here, I know that the __dirname vs import.meta.dirname thing keeps tripping people up quite a bit.

"got": "^11.7.0",
"graceful-fs": "^4.2.11",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Appreciate it, thank you

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants