airbnb / airbnb/react-sketchapp

Proposal: making `render()` async

Open
#469 26 comments 0 reactions 0 assignees View on GitHub
Dominant language
TypeScript
Stars
14.9k
Forks
797
PR merge metrics
No merged PRs in 30d

Description

As we discussed [in the PR over node-sketch-bridge](https://github.com/Lona/node-sketch-bridge/pull/6#issuecomment-575070941), I'm working on writing a "pure" implementation of `makeImageDataFromUrl()`, using `fetch` and `@skpm/fs`.

There is one major problem implementing it in this way though: `fetch` is asynchronous, while the whole rendering operation is synchronous.

Now, I see two ways around this:
1. I keep pieces of the existing implementation that uses `NSData` around, and keep everything sync. Not surprisingly, [NSData documentation](https://developer.apple.com/documentation/foundation/nsdata/1413892-init) has a big fat warning about not using it for network requests, though, because it can potentially hang the whole thing if the network is slow/unresponsive. The pro is that I can keep the existing interface. The con is that, as per current implementation, it can potentially hang Sketch, and the implementation is less uniform.
2. Make `makeImageDataFromUrl` return a Promise and make the whole chain of functions up to `render` async as well. This allows us to use `fetch`, makes the whole thing faster in cases where there are multiple images in a layout, and in general less prone to hang, but it's a breaking change.

@mathieudutour Thoughts?

Contributor guide

Open the contributing guide

Research direction

Begin with makeImageDataFromUrl and render, then read the linked node-sketch-bridge PR and this discussion to determine whether the rendering chain should become Promise-based. Done requires a recorded API decision and agreed implementation scope for the breaking change.

Written by the indexing model from the issue text.

Assessment

Tech stack
react, typescript
Domain
desktop, frontend
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.