search

wbyoung / babel-plugin-transform-postcss

Babel PostCSS Transform
MIT License
55 stars 23 forks source link

Simplify code base by using deasync #2

Open wbyoung opened 7 years ago

wbyoung commented 7 years ago

This addresses the client-server concerns from #1 by using deasync to completely remove the client-server setup. I'm a little wary about how deasync achieves its goal, but have looked a fair amount into possible improvements including replacing use of some private process APIs. To that end, I've opened https://github.com/abbr/deasync/pull/75.

This is, however, a build-only tool & should never really be used in production. So if for some reason something goes wrong because of this private API use, it's far less devastating in during a build than in production.

coveralls commented 7 years ago

Coverage Status

Coverage remained the same at 100.0% when pulling 6a6856ddc0e59c918cf2707e42da0a06ce8cc196 on deasync into 209ce2337e1c78813c744811aedb47fe2d37ff1c on master.

gajus commented 7 years ago

I was looking into deasync too to solve an equivalent problem. https://github.com/gajus/babel-plugin-react-css-modules/issues/83

I'd be curious to know what are the performance implications. As you said, this is a built-time only tool, though I wouldn't like my build tool to consume 100% CPU when idle (which, correct me if I am wrong, would be the result of consumingdeasync).

wbyoung commented 7 years ago

@gajus no, it wouldn't consume 100% CPU while idle. First, deasync is a busy loop only in the sense that it while (!done) loops, but it's running the event loop during each check, so it's actually really only busy waiting while other things are going on. Second, and more relevant to your question, this would only run when the plugin is processing an import statement — only when you're compiling your code w/ Babel.

chrisngobanh commented 7 years ago

What is blocking this PR? I hope to see this change implemented because the plugin seems unnecessarily bulky with the server-client set up.

wbyoung commented 7 years ago

@chrisngobanh discussion & testing really. I'm a little nervous about it because I'm not crazy about hacking at Node.js the way that deasync is doing in order to get things working. It's not the most future-proof solution, and requires deep understanding of v8/Node.js in order to write those few little lines of C++ code.

Honestly, the fluctuations in how events work between recent versions of Node (with some focus on Promise related features) has been decently large. Even using NaN doesn't future-proof it because it's running the event loop manually.

Things like abbr/deasync#82 concern me as well… just having that in the dependency chain and having to ensure that the project re-builds as required for Node.js releases.

I'm just struggling with how worthwhile it'd be.

Honestly, abstracting the client/server piece into a standalone project may simplify the code base nice enough.

Thoughts?