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

[RootView] Asynchronously load the bundle to give time to configure the root view #291

Closed
wants to merge 1 commit into from

Conversation

ide
Copy link
Contributor

@ide ide commented Mar 27, 2015

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

@ide
Copy link
Contributor Author

ide commented Mar 27, 2015

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.

@amasad
Copy link
Contributor

amasad commented Mar 27, 2015

Any noticeable speed improvements?

@ide
Copy link
Contributor Author

ide commented Mar 27, 2015

@amasad This actually introduces a delay of one run loop of the main thread (should be under 16ms) to fix a bug.

@nicklockwood
Copy link
Contributor

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.

@ide
Copy link
Contributor Author

ide commented Mar 30, 2015

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?

@vkurchatkin
Copy link
Contributor

+1 on keeping it. It could be a way to reuse 3rd party JavaScriptCore code unaware of React Native

@tadeuzagallo
Copy link
Contributor

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.
But we can look into that...

@ide
Copy link
Contributor Author

ide commented Mar 31, 2015

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
@vjeux
Copy link
Contributor

vjeux commented Apr 3, 2015

setScriptURL doesn't exist anymore. Can you re-open if it is still useful.

@vjeux vjeux closed this Apr 3, 2015
@ide
Copy link
Contributor Author

ide commented Apr 3, 2015

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.

mganandraj pushed a commit to mganandraj/react-native that referenced this pull request Apr 14, 2020
…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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[RootView] RCTRootView initializer has problems when configuring the executor
6 participants