search

contentsync / SketchContentSync

Sync sketch files with google docs.
275 stars 17 forks source link

Sketch hangs if symbol ID from sync is not in Sketch file #127

Closed dbrody closed 3 years ago

dbrody commented 3 years ago

Issue submitted by other user:

"My ContentSync sync operations never complete if a symbol ID is referenced in the sheet, but does not exist in Sketch – the only way out is to hard-quit Sketch. It seems like something is entering an infinite loop because my CPU gets maxed out."

nilsdoehring commented 3 years ago

Hi @dbrody here are steps to reproduce in v8.3.1:

Result:

This issue can be narrowed down to the symbol type IDs of the Spanish version, Símbolo-Definición-1 and Símbolo-Definición-2. In the Sketch file, no such symbols do exist. Once symbols of these names are created, or Version-1 is selected, the Pull runs in just seconds without any issues.

dbrody commented 3 years ago

@nilsdoehring Great, thank you for these details!

nilsdoehring commented 3 years ago

@dbrody thank you for your patience. This behaviour also occurs when the symbol is part of a library (i.e. technically, the symbol does exist, but not within the Sketch file from which the pull is run from). Once an instance of such a symbol is placed within that Sketch file, the Pull runs fine. Does ContentSync support replacing with symbols from libraries?

dbrody commented 3 years ago

@nilsdoehring We are working to address this issue.

An initial improvement has been released in our 8.4.0-alpha release. Please note that it is an alpha release so it has not gone under heavy stress testing. We'd love feedback as we work towards a production release of these improvements.

nilsdoehring commented 3 years ago

@dbrody with 8.4.0-alpha, the behaviour described above changed slighty:

So it seems while 8.4.0-alpha successfully resolves #131 , the symbol ID issue described here has not yet been addressed. By the way: The granular status updates in the side bar are great!

dbrody commented 3 years ago

@nilsdoehring

  1. The beach ball is expected for short moments since we need to hang the UI to make updates to Sketch layers. While this should be very quick on a small update, on a huge file update it will be noticeable. However we have moved a large part of this to the background so Sketch should be responsive for a large part of the time a sync is happening.
  2. Yes, we will correct that!
dbrody commented 3 years ago

@nilsdoehring Relase 8.4.1-alpha should fix this. It fixes and improves performance of fetching remote libraries and now will honor enabled/disabled libraries as well. Hope that helps.

nilsdoehring commented 3 years ago

@dbrody I can confirm that 8.4.1-alpha resolves this issue.

I'm wondering though: The release notes say Fixes searching libraries to ignore disabled libraries – but in my tests with 8.4.1-alpha, image symbol overrides that point to a library just do not get replaced in a pull. Only after an instance of the image symbol from the library is inserted anywhere in the Sketch file, the override gets replaced successfully.

I think the reason is how the Sketch API deals with library symbol imports.

Would this belong to this issue or should I file a feature request?

dbrody commented 3 years ago

Best to create a new issue for that. It sounds like this issue is resolved so I'd like to keep them separate. Thanks!

nilsdoehring commented 3 years ago

Sure! (#132)
Thank you so much for resolving this issue!

Am I right in assuming that before, more logic was bound to API calls to the Google API, while now, ContentSync pulls the whole sheet with a single-ish API call, internalizing most of the others?

dbrody commented 3 years ago

Great, I'll close this issue.

In terms of fixing this item? It had more to do with memoizing long running actions and moving some of the updates to background threads. Part of the benefit of this upgrade is the updates now make better use of the OSX hardware by utilizing multiple threads.