-
Notifications
You must be signed in to change notification settings - Fork 24.4k
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
[RootView] Asynchronously load the bundle to give time to configure the root view #291
Conversation
cc @nicklockwood I believe you are the right person to look at this. it's a quick fix but shouldn't have adverse side effects until an API change can be worked out. |
Any noticeable speed improvements? |
@amasad This actually introduces a delay of one run loop of the main thread (should be under 16ms) to fix a bug. |
This makes sense, but I think we're ditching the public property for setting executor, since you'll only ever want the JSCore one for deployment, or the WebSocket/WebView one for debug. Might add a private setter for injecting a mock version (for unit testing), but otherwise there seems little benefit to exposing the class. |
I actually use a custom executor (similar to the ContextExecutor) to set up the JS environment so could you keep a way to set it? |
+1 on keeping it. It could be a way to reuse 3rd party JavaScriptCore code unaware of React Native |
Right now you still can set the class it will use, but not prior to running... You have to set it, and then reload, and it can't be at initialization time, or the bridge will ignore you. |
This diff addresses that issue by giving the initial load one pass of the run loop to set up the executor class. It's not meant to be a permanent solution but the patch is kept tidy in one place and shouldn't add maintenance cost until another solution is in place. |
…he root view If you construct an RCTRootView you may want to configure the executor. However the constructor synchronously calls `loadBundle` and sets up the executor and bridge. This is a quick fix that uses dispatch_async to allow the current pass of the runloop time to set up the executor. Fixes facebook#288
setScriptURL doesn't exist anymore. Can you re-open if it is still useful. |
This is still an issue -- RCTBridge calls setUp from its initializer, which synchronously creates the executor. I'm not able to reopen GH issues but have updated the branch/diff. |
…t-native init` projects. (facebook#291) * Got npx command line working. * Generating project folder. * Added macOS template files. * Lint fixes * Remove temp workarounds. * Make react-native.config.js mac and windows compatible. * Restore parity of macOS components with iOS because the `react-native init` sample app depends on them: e.g. StatusBar. * Updated NewAppScreen language for macOS. * Added `--prerelease` switch to allow installing pre-rerelease versions without prompting as this will be needed in CI. Made tweaks to templates and fixed template schemes to rename to project name.
If you construct an RCTRootView you may want to configure the executor. However the constructor synchronously calls
loadBundle
and sets up the executor and bridge. This is a quick fix that uses dispatch_async to allow the current pass of the runloop time to set up the executor.Tested by constructing an RCTRootView and then customizing the executor class on the next line. Works with this patch.
Fixes #288