Add bundle progress bar to logs #140
Conversation
react-native-scripts/package.json
Outdated
| "qrcode-terminal": "^0.11.0", | ||
| "xdl": "^37.0.0" | ||
| "xdl": "38.0.0-beta.2" |
There was a problem hiding this comment.
I'm assuming we'll release this as at-latest before we merge this?
| @@ -33,8 +33,9 @@ | |||
| "match-require": "^2.0.0", | |||
| "minimist": "^1.2.0", | |||
| "path-exists": "^3.0.0", | |||
| "progress": "^1.1.8", | |||
There was a problem hiding this comment.
This issue concerns me re: windows support
There was a problem hiding this comment.
indeed, I don't have a windows machine to test this :<
There was a problem hiding this comment.
or rather I do but I have no idea how to use windows for anything except oculus and netflix
There was a problem hiding this comment.
fwiw we use this same ProgressBar module on exp so this could be a problem we have to address there too if it is indeed true and not user error in that issue
There was a problem hiding this comment.
ah ok. hm maybe it's broken in exp too then?
There was a problem hiding this comment.
can you try out installing exp and init'ing a project? should show progress bar while downloading
There was a problem hiding this comment.
wipes tear away while spinning up a VM
There was a problem hiding this comment.
so apparently the inquirer package is also irrevocably broken on git bash, so i'm just going to step away slowly
There was a problem hiding this comment.
resolution: don't support git bash for now if you want a pleasant crna experience. powershell recommended on windows for now. if anyone reading this is interested in fixing git bash support it would be welcome!
| return; | ||
| } | ||
| if (packagerReady) { | ||
| const message = `${chunk.msg.trim()}\n`; |
There was a problem hiding this comment.
such template literal much wow
react-native-scripts/src/util/log.js
Outdated
|
|
||
| let _bundleProgressBar; | ||
|
|
||
| function respectProgressBars(commitLogs) { |
There was a problem hiding this comment.
The existence of this function makes me think that we also need to replace console.log's in other scripts, as there's a chance we may use, for example, the Expo login stuff inside of a script that needs to respect progress bars.
There was a problem hiding this comment.
Thinking about this more, we probably want to replace all of the console.log statements so that there's no confusion about when or where to use this?
There was a problem hiding this comment.
@dikaiosune - so just:
let _oldConsoleLog = console.log;
console.log = log;
// implement log with _oldConsoleLog
brentvatne
force-pushed
the
@brent/add-bundle-progress-bar
branch
from
e80f488
to
ecdf6cb
Compare
|
ok I think I addressed everything. you can test this out by cloning and running |
| @@ -1,5 +1,5 @@ | |||
| { | |||
| "expo": { | |||
| "sdkVersion": "15.0.0" | |||
| "sdkVersion": "UNVERSIONED" | |||
There was a problem hiding this comment.
of course we will change this before merging, just changed it for now so you can test
|
we can almost merge this @dikaiosune ;) |
|
Woooooooo Is it just waiting on sdk 16? |
|
Not sure why it is showing changes vs. master that I rebased into the branch, but I'm testing this now. |
anp
changed the title
| console.log(`Installing dependencies using ${command}...`); | ||
| console.log(); | ||
| log(`Installing dependencies using ${command}...`); | ||
| log(); // why is this here |
There was a problem hiding this comment.
because it's ugly otherwise and i apparently forgot how newlines work
It works, see this image. Depends on facebook/react-native#13172
Falls back to this on old SDKs