Use async/await in Rollup scripts #11669
Conversation
| await createBundle(bundle, FB_DEV); | ||
| await createBundle(bundle, FB_PROD); | ||
| await createBundle(bundle, RN_DEV); | ||
| await createBundle(bundle, RN_PROD); |
There was a problem hiding this comment.
Woah. I've never seen await in a loop before. 😻
| return Promise.resolve(); | ||
| } | ||
| // Run them serially for better console output | ||
| // and to avoid any potential race conditions. |
There was a problem hiding this comment.
This comment might be nice to move up to line ~446, for the whole rimraf callback, since we also execute the first few create* tasks serially.
Edit: Actually now that I look again, I'm pretty sure we could just leave the comment here and parallelize the above few tasks 😄 We were running them in serial before but I don't think those need to be. (Not that it would make a bit difference. Those tasks should be super fast.)
There was a problem hiding this comment.
I don't mind parallelizing them but I'm not 100% sure they're free of race conditions. I wouldn't both anyway tbh since they're really fast as you said.
| if (syncFbsource) { | ||
| await syncReactNative(join('build', 'react-native'), syncFbsource); | ||
| await syncReactNativeRT(join('build', 'react-rt'), syncFbsource); | ||
| await syncReactNativeCS(join('build', 'react-cs'), syncFbsource); |
There was a problem hiding this comment.
nit: We could wrap these in a Promise.all (not that it would make a big diff, should be fast either way)
There was a problem hiding this comment.
The build script originally did exactly this but it made the output unreadable (the different bundle errors/warnings were spat out over each other) and there win wasn't actually that of a win in performance as the work is mostly CPU intensive, not IO intensive.
There was a problem hiding this comment.
That's for the create-bundle steps (below). The sync steps here just copy files. They should be okay to do in parallel. (They don't even log anything, so they wouldn't cause unreadable output.)
But, it wouldn't make a big diff in terms of performance since they're just copying a few files. So it's a "nit" 😄
Removes some boilerplate since we depend on Node 8 anyway.
Best viewed with
?w=1.