oodesign / merge-duplicate-symbols

Sketch plugin to merge symbols and layer&text styles.
https://www.mergeduplicates.com/
278 stars 15 forks source link

Silently fails in some files #20

Closed kty closed 6 years ago

kty commented 6 years ago

Some people already reported this. In some of my files (large projects with many pages), plugin wasn't doing anything without showing any error. I tried it both in a regular and a library file.

I narrowed the problem down to a symbol which was duplicated from 2 different libraries. So, I had a local symbol using a library symbol called "Color/Primary" from library A, and somewhere else, another symbol using a library symbol called "Color/Primary" from library B.

I didn't prepare a test file though because I managed to fix the code. It works, but I have no experience in Cocoascript and don't know what's the core issue, so I'll just paste it here in case it's helpful.

After running the plugin, I get this in Sketch dev tools:

TypeError: symbolID.localeCompare is not a function. (In 'symbolID.localeCompare(symbolMaster.symbolID())', 'symbolID.localeCompare' is undefined)
line: 113
sourceURL: /Users/tomoki/Library/Application Support/com.bohemiancoding.sketch3/Plugins/merge-duplicate-symbols-2.0/MergeDuplicateSymbols.sketchplugin/Contents/Sketch/MergeDuplicateSymbols.cocoascript
column: 39
stack: FindOverrideSymbolID@/Users/tomoki/Library/Application Support/com.bohemiancoding.sketch3/Plugins/merge-duplicate-symbols-2.0/MergeDuplicateSymbols.sketchplugin/Contents/Sketch/MergeDuplicateSymbols.cocoascript:113:39
getSymbolOverrides@/Users/tomoki/Library/Application Support/com.bohemiancoding.sketch3/Plugins/merge-duplicate-symbols-2.0/MergeDuplicateSymbols.sketchplugin/Contents/Sketch/MergeDuplicateSymbols.cocoascript:98:24
FindSymbolOverrides@/Users/tomoki/Library/Application Support/com.bohemiancoding.sketch3/Plugins/merge-duplicate-symbols-2.0/MergeDuplicateSymbols.sketchplugin/Contents/Sketch/MergeDuplicateSymbols.cocoascript:55:49
onRun@/Users/tomoki/Library/Application Support/com.bohemiancoding.sketch3/Plugins/merge-duplicate-symbols-2.0/MergeDuplicateSymbols.sketchplugin/Contents/Sketch/MergeDuplicateSymbols.cocoascript:302:49

I tried logging the symbolID right before that line:

<MOMethod: 0x60400b030a20 : target=0x60400cc3e9e0{
    symbolID = "A09CEDD4-44EC-4EB8-8BA3-1327DE7EF9F1";
}, selector=symbolID>

Turns out it's a function, not a string, so all I had to do was add this (in line 113):

if (typeof symbolID === 'function') {
    symbolID = symbolID()
}

It's super dirty, but at least it works. Hope it's helpful. :)

oodesign commented 6 years ago

Wow, what a bug! :D As far as I know, that symbolID is not supposed to be a method, but a string... but I'll double check on that. May I ask you which version of Sketch and of the MergeDuplicateSymbols are you using?

Further than this, we've tried to reproduce this very same situation (a Sketch file using two symbols with the same name that como from two different libraries), and we still get that symbolID as a string. Even creating a nested symbol with those symbols in it and overriding them (which is what leads to that piece of code) doesn't seem to return any method...

May you send us your Sketch file and the referenced libraries so that we can test it on our own?

Thank you very much for your time, and for investigating this issue. We really appreciate.

kty commented 6 years ago

I'm afraid I can't send the file (I'm bound by NDA), but I might be able to prepare a simplified case.

I use the latest versions of both, Sketch 49.1 and MergeDuplicateSymbols 3.0.4.

oodesign commented 6 years ago

Sure, we just need the symbols. You may replace their content for dummy rectangles, if you like to. Thanks!

oodesign commented 6 years ago

Hi!

We just released version 3.0.6, that hopefully would solve some of the issues causing the plugin to do nothing apparently in several situations.

Please, let us know if it did work for you!