Skip to content
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

Multiple commands in one file break Sketch #33

Closed
cameronmcefee opened this issue Apr 19, 2017 · 14 comments
Closed

Multiple commands in one file break Sketch #33

cameronmcefee opened this issue Apr 19, 2017 · 14 comments

Comments

@cameronmcefee
Copy link

Given a command file test.js containing:

export function foo(){}
export function bar(){}

the output will be:

Object.defineProperty(exports, '__esModule', { value: true });

function foo() {}
function bar() {}

exports.foo = foo;
exports.bar = bar;

However, Sketch doesn't have an exports variable, so this fails. The problem doesn't happen when using export default because the result compiles to var onRun. This means that it isn't possible to hold multiple commands in a single file without some hackery. To work around it, one can do this:

var foo = function (){}
var bar = function (){}

export default function(){
  foo, bar
}

which compiles to:

var foo = function () {};
var bar = function () {};
var onRun = function () {
  foo, bar;
};

onRun can be ignored now that foo and bar are accessible. However, it must be included to ensure something is compiled (the file will be empty otherwise) and the references to foo and bar inside it trick the compiler into thinking they're needed, thus including them.

@cameronmcefee cameronmcefee changed the title multiple commands in one file break Sketch Multiple commands in one file break Sketch Apr 19, 2017
@mathieudutour
Copy link
Member

yes, I fixed that when we were using rollup but switching to webpack broke it.

Can you tell me what is your use-case? I had one in mind specifically (onClick handler for react-sketchapp) but I'm curious if there is more

@cameronmcefee
Copy link
Author

cameronmcefee commented Apr 19, 2017

I have a handful of commands that I'm placing in the Sketch menu which are contextually similar, but call different methods (ex "Clear all guides", "Clear guides in artboard", "Clear horizontal guides", etc.) They're all effectively one liners into other methods so it seems gratuitous to put them all in their own separate files. I suppose one could argue that I make the whole thing a class and export that.

The last time I ran it, I was looking for a solution for syncing the current context with your sketch-web-view. I had two commands, one that launched the web view and one that was a listener for selection change events which would sync data into the html content of the webview. I ultimately opted for splitting them apart because they were different enough.

@mathieudutour
Copy link
Member

got it. I'll try to find a fix

@cameronmcefee
Copy link
Author

cameronmcefee commented Apr 19, 2017

No rush on my end. The hack works fine for me, but I figured I'd report it in case it's something you care about, and because it does make for some slightly unpredictable results.

@mathieudutour
Copy link
Member

I actually don't know how to do it. Is there any webpack expert around who have an idea?

@cameronmcefee
Copy link
Author

cameronmcefee commented Apr 25, 2017

For reference, here's the actual error in Console.app

ReferenceError: Can't find variable: exports.
Plugin “All artboard guides”, line 43:
» Object.defineProperty(exports, '__esModule', { value: true });

For giggles, I tried doing this:

var exports
export var foo = function (){}
export var bar = function (){}

export default function(){
  exports
}

But that gets compiled in a way that gives exports a new name so it doesn't clobber the actual exports variable, which I suppose is good behavior.

I guess the only solution I can picture is something that just prepends var exports to the file, but I'm not sure that's any better than my hack mentioned in the OP

@mathieudutour
Copy link
Member

thinking about this a bit more: would adding var exports at the top of the file with a webpack plugin (https://webpack.js.org/plugins/banner-plugin/) make it work?

@cameronmcefee
Copy link
Author

Hmm… interesting idea. I scrapped this project in favor of a cleaner solution, but I'll try this out once I get back to this stage.

@mathieudutour
Copy link
Member

I manage to do it here: 804046e

@rborn
Copy link

rborn commented Jun 29, 2017

I can confirm it works 👍

@mathieudutour
Copy link
Member

That was quick! Thanks

@rborn
Copy link

rborn commented Jun 29, 2017

@mathieudutour could you please bump the version and push to npm? 🤗

@mathieudutour
Copy link
Member

It's a breaking change, I'd like to get #44 in as well, and maybe fixing #45 before releasing

@rborn
Copy link

rborn commented Jun 29, 2017

Ok, I can live with the github version, thanks!

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

No branches or pull requests

3 participants