Skip to content

fix: fall back to darwin_x64 if available on darwin_arm64 #69

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

Merged
merged 1 commit into from
Dec 14, 2020

Conversation

sgtcoolguy
Copy link
Contributor

This is a simple fallback mechanism. There's no Darwin_arm64 binary built here yet, so on those machines this will fall back to the Darwin_x64 binary if available (which it is). Works for me locally on a new macMini M1.

Copy link
Contributor

@mprobst mprobst left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! One nit.

index.js Outdated
'Consider installing it with your native package manager instead.\n';
throw new Error(message);
if (platform === 'darwin' && arch === 'arm64') {
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe we could write this a little less repetitively?

E.g.

if (existsSync(nativeBinary)) return nativeBinary;
if (darwin && arm64) { check if darwin_x64 exists, return if so }
construct error message and throw

That'd avoid the duplicate throw, and also move all error msg construction and handling after the positive paths.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mprobst Ok, I've force-pushed a new commit which is a little cleaner and follows the early return suggestion here.

@sgtcoolguy sgtcoolguy force-pushed the darwin-arm64-fallback branch from dde3a06 to 49b2fde Compare December 14, 2020 14:31
@mprobst mprobst merged commit ea44b58 into angular:master Dec 14, 2020
@mprobst
Copy link
Contributor

mprobst commented Dec 14, 2020

Thanks!

@mprobst
Copy link
Contributor

mprobst commented Dec 14, 2020

Released in 1.5.0.

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

Successfully merging this pull request may close these issues.

3 participants