Skip to content
This repository has been archived by the owner on Jan 18, 2024. It is now read-only.

fix: make sharp image processing library an optional dependency #742

Merged
merged 3 commits into from
Jun 19, 2019

Conversation

fson
Copy link
Contributor

@fson fson commented Jun 18, 2019

Why?

Installing the sharp image processing library can fail for a few resons:

When sharp installation fails, previously the whole Expo CLI installation has failed.

How

To make it possible to install Expo CLI without sharp, this PR makes sharp an optional dependency (via the sharp-cli package). Only certain functionality (expo optimize and web builds) require sharp.

For most users sharp will be installed without problems, and those users will be able to use all features without any additional setup.

Those users who previously had problems installing sharp will continue to see warnings when installing but because the dependency is now optional, it won’t fail the whole installation. They will also be able to use Expo CLI. When trying to use functionality that requires sharp, Expo CLI will prompt them
to separately install the sharp-cli package globally. After sharp-cli is installed, those commands can be used.

Screenshot 2019-06-18 at 20 21 33

Screenshot 2019-06-18 at 20 22 23

Further improvements

In the future, we could make our own distribution of sharp to fix the permissions issues (using the same method we use for ngrok and traveling fastlane). Additionally we could provide JS-only fallbacks (even if they’re slower) for platforms that sharp doesn’t support.

@fson fson requested review from ide and brentvatne June 18, 2019 22:41
};

type Options =
| {}
Copy link
Member

Choose a reason for hiding this comment

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

Why do we need this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

TypeScript complained that {} is not compatible with Options. (The options for the removeAlpha operation are {}.)


const SHARP_HELP_PATTERN = /\n\nSpecify --help for available options/g;

export async function sharpAsync(
Copy link
Member

Choose a reason for hiding this comment

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

Specify return values in TypeScript too?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍

// Delete the optimized version and revert changes
fs.renameSync(newName, image);
if (amountSaved > 0) {
await fs.move(image, newName);
Copy link
Member

Choose a reason for hiding this comment

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

Promise.all

Copy link
Contributor Author

@fson fson Jun 19, 2019

Choose a reason for hiding this comment

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

These operations should not happen in parallel – we first move the old file to filename.orig.ext and then move the optimized image from a temporary folder to its place.

@fson fson changed the title fix: make sharp image processing library an optional depdency fix: make sharp image processing library an optional dependency Jun 19, 2019
@fson fson merged commit 60cdbc7 into master Jun 19, 2019
@fson fson deleted the @fson/optional-sharp branch June 19, 2019 17:09
@EvanBacon EvanBacon mentioned this pull request Sep 15, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants