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

Javascript host environment documentation #335

Closed
aackerman opened this issue Mar 27, 2015 · 34 comments
Closed

Javascript host environment documentation #335

aackerman opened this issue Mar 27, 2015 · 34 comments
Labels
Resolution: Locked This issue was locked by the bot.

Comments

@aackerman
Copy link

What I was lost on earliest attempting to get started with react-native was understanding the host environment. It's not Node, it's not a browser, it appears to be a raw VM context, with a whole bunch of polyfills. When I tried to require Bluebird, I ran into errors, likely due to assumptions made about APIs provided by the host environment.

Also, it would be important to know how third-party libs can run tests in the same type of environment to fix these issues.

Thanks.

@jaygarcia
Copy link
Contributor

I agree that there could be some better documentation around this. For example, the "light" implementation of the CommonJS module system, should be documented, if it's not already.

(stuff is moving so quickly, it's hard for me to keep track of it all)

@daviskoh
Copy link
Contributor

+1

@sahrens
Copy link
Contributor

sahrens commented Mar 27, 2015

We always need more docs!

For testing in the full environment, check out the "Testing" section of the documentation - we have a harness for integration testing that should help you do what you want.

On Mar 27, 2015, at 7:38 AM, Davis Koh @daviskoh notifications@github.com wrote:

+1


Reply to this email directly or view it on GitHub.

@amasad
Copy link
Contributor

amasad commented Mar 27, 2015

@aackerman can you list the issues you ran into by trying to use Bluebird? Maybe we can fix those quickly.

@aackerman
Copy link
Author

I can give more detailed info about the issue with bluebird later tonight, but in the mean time, having installed bluebird with npm install bluebird and var Promise = require('bluebird'); in index.ios.js will throw and error.

@vjeux
Copy link
Contributor

vjeux commented Mar 27, 2015

Can you give the exact error?

Also note that there's a Promise global already existing in React Native

@aackerman
Copy link
Author

It looks like a clear issue with bluebird from what I can tell.

screen shot 2015-03-27 at 18 21 29
screen shot 2015-03-27 at 18 21 57

The block in bluebird looks like this.

} else if (typeof MutationObserver !== "undefined") {
    schedule = function(fn) {
        var div = document.createElement("div");
        var observer = new MutationObserver(fn);
        observer.observe(div, {attributes: true});
        return function() { div.classList.toggle("foo"); };
    };
    schedule.isStatic = true;
} else if (typeof setTimeout !== "undefined") {

It's clearly throwing an error because the DOM is not available, but it's interesting that MutationObserver is available, otherwise it could not have made it into the block. The authors of bluebird probably didn't expect to be running in an environment that is neither node nor the browser.

@vjeux as you said a Promise implementation does exist already, and what I really wanted just simply fetch, so the bluebird issue is completely secondary to understanding the host environment.

I'll make a ticket on the bluebird repo to let them know about this particular issue, but at this point I don't think it needs to be a priority for anybody.

@vjeux
Copy link
Contributor

vjeux commented Mar 27, 2015

Yeah, all those feature detection needs to be carefully considered when running in a different environment. The implementation of Promise that we have is using setImmediate that has been implemented to run before we hand back the process to obj-c

@vjeux
Copy link
Contributor

vjeux commented Mar 27, 2015

By the way, fetch is there by default. It has a much nicer api than xhr :)

@petkaantonov
Copy link

MutationObserver is supposed to observe DOM elements, it doesn't make sense to expose one without the other. Also, there is not a single mention of MutationObserver in this entire repo so I am not sure how I would even fix it - what am I supposed to observe with it?

@vjeux
Copy link
Contributor

vjeux commented Mar 28, 2015

This is where we mangle with the global environment. We should probably do delete GLOBAL.MutationObserver; (if we can).
https://github.com/facebook/react-native/blob/master/Libraries/JavaScriptAppEngine/Initialization/InitializeJavaScriptAppEngine.js#L117

One issue if we do this is that it's now going to use setTimeout(0), which is going to do a round trip to native for every single call. Since when you are using promises, you tend to chain a lot of then, it's going to be much slower than needed. It really needs to use setImmediate.

@petkaantonov
Copy link

I can add setImmediate check after the MutationObserver check, np :)

@vjeux
Copy link
Contributor

vjeux commented Mar 28, 2015

Okay perfect. I'm trying to remove MutationObserver right now, I'll report back in 2 minutes

@vjeux
Copy link
Contributor

vjeux commented Mar 28, 2015

Bingo, it's working, it now falls through the setTimeout check. I'm sending an internal diff, it'll be updated in the next version.

@petkaantonov
Copy link

setImmediate check published in 2.9.16

@vjeux
Copy link
Contributor

vjeux commented Mar 28, 2015

That was fast ;)

@petkaantonov
Copy link

yea the upside of lone cowboy projects :p

@vjeux
Copy link
Contributor

vjeux commented Mar 28, 2015

Since I got you, I'd like to get your opinion on
https://github.com/facebook/react-native/blob/master/Libraries/vendor/core/ES6Promise.js

This is the promise implementation that we're providing. Do you think it makes sense to use bluebird by default?

@petkaantonov
Copy link

Well I don't personally find the pure ES6 api enough for application development, I find stuff like .finally, predicated catches, long stack traces, unhandled rejection reporting etc essential. Also in 3.0 bluebird debuggability is going to be turbo boosted by warnings that non-seasoned promise users commonly make, e.g. https://twitter.com/PetkaAntonov/status/565899315613540352

@vjeux
Copy link
Contributor

vjeux commented Mar 28, 2015

Yeah, at Facebook we use a slightly improved version of Promises with .finally and .done

https://github.com/facebook/react-native/blob/master/Libraries/vendor/core/Promise.js#L42

Don't have the rest though. I'll poll people internally and see what they think.

@aackerman
Copy link
Author

The warnings about uncaught promise rejections are the number 1 reason to use bluebird over the specced ES6 promises. Trying to find silently swallowed errors is developer hell.

@aackerman
Copy link
Author

If I was intent on finding out more about the JS host environment is here the best place to look?

@aackerman
Copy link
Author

Awesome thanks, I'll do some reading and see if I can distill some of the information.

@petkaantonov
Copy link

btw I checked the finally implementation - just a note that .finally in bluebird doesn't behave like .then(cb, cb); instead it behaves like this

@benjamingr
Copy link

@vjeux I think you really want to have long stack traces, unhandled rejection detection and better aggregation methods in your promises. You can always just use a special build of bluebird.

Facebook already has groups using bluebird internally (namely WhatsApp Web), I think having a robust promise implementation would be really nice for react native. (That said, things like a TableView would matter a lot more to me as a user :P)

@vjeux
Copy link
Contributor

vjeux commented Mar 29, 2015

Is there a special build of bluebird that's compliant with the es6 spec? Aka not adding extensions such as finally?

@benjamingr
Copy link

@vjeux you can remove extensions if you really want to beyond the most partial build (or just not use them) but why? Stuff like .finally will eventually get included in ES, the only reason it was not already included is because they didn't have tie (I can dig up the thread if you'd like).

You can get bluebird down in a partial build to ~5kb but to be fair the extra stuff that makes bluebird bigger has huge usability gains.

@petkaantonov
Copy link

I'm not sure I see the point of such a build - there is literally thousands of es6-promise shims available already

@benjamingr
Copy link

@petkaantonov well, the general idea is to get the wins from bluebird like long stack traces and more debuggable and faster promises in react-native I think.

@petkaantonov
Copy link

Those are not compliant with es6- spec

@benjamingr
Copy link

So what? React is not world renowned for sticking to strict ES6 it's known for making pragmatic choices in order to make developer life easier. Adding long stack traces and enabling debuggable promises would be a huge deal.

@vjeux
Copy link
Contributor

vjeux commented Mar 29, 2015

React native environment should look like the browser / node. By putting non standard primitives as global we would break all the existing code out there. This would be bad for the entire ecosystem.

In this case, people can just npm install bluebird to get all those non standard behavior if they want to

vjeux added a commit to vjeux/react-native that referenced this issue Apr 13, 2015
…detection

Summary:
Bluebird is checking if MutationObserver exists and if it does, uses it and instant crashes. Since we don't have the DOM, MutationObserver doesn't make sense.

Also, pretty cool thing, the bluebird maintainer updated the feature detection to fallback to setImmediate before checking setTimeout. He committed and made a new version in less than 5 minutes!

facebook#335 (comment)

Test Plan: Run bluebird and make sure it runs with setImmediate
@brentvatne
Copy link
Collaborator

I believe that this is covered in this doc now: https://facebook.github.io/react-native/docs/javascript-environment.html#content

facebook-github-bot pushed a commit that referenced this issue Jan 26, 2017
Summary:
Followup of #335, fix #326. This commit add the `YGLayoutGetBorder(node, edge)` function, which correctly takes RTL/LTR into account when resolving `EDGE_START` & `EDGE_END`.
Closes facebook/yoga#344

Reviewed By: dshahidehpour

Differential Revision: D4459950

Pulled By: emilsjolander

fbshipit-source-id: b57eb7a5b1c181a364913c3200a3794a2b7b31a6
edmofro pushed a commit to edmofro/react-native that referenced this issue Feb 6, 2017
Summary:
Followup of facebook#335, fix facebook#326. This commit add the `YGLayoutGetBorder(node, edge)` function, which correctly takes RTL/LTR into account when resolving `EDGE_START` & `EDGE_END`.
Closes facebook/yoga#344

Reviewed By: dshahidehpour

Differential Revision: D4459950

Pulled By: emilsjolander

fbshipit-source-id: b57eb7a5b1c181a364913c3200a3794a2b7b31a6
nicktate pushed a commit to nicktate/react-native that referenced this issue Feb 7, 2017
Summary:
Followup of facebook#335, fix facebook#326. This commit add the `YGLayoutGetBorder(node, edge)` function, which correctly takes RTL/LTR into account when resolving `EDGE_START` & `EDGE_END`.
Closes facebook/yoga#344

Reviewed By: dshahidehpour

Differential Revision: D4459950

Pulled By: emilsjolander

fbshipit-source-id: b57eb7a5b1c181a364913c3200a3794a2b7b31a6
normanjoyner pushed a commit to nicktate/react-native that referenced this issue Feb 9, 2017
Summary:
Followup of facebook#335, fix facebook#326. This commit add the `YGLayoutGetBorder(node, edge)` function, which correctly takes RTL/LTR into account when resolving `EDGE_START` & `EDGE_END`.
Closes facebook/yoga#344

Reviewed By: dshahidehpour

Differential Revision: D4459950

Pulled By: emilsjolander

fbshipit-source-id: b57eb7a5b1c181a364913c3200a3794a2b7b31a6
nicktate pushed a commit to nicktate/react-native that referenced this issue Feb 9, 2017
Summary:
Followup of facebook#335, fix facebook#326. This commit add the `YGLayoutGetBorder(node, edge)` function, which correctly takes RTL/LTR into account when resolving `EDGE_START` & `EDGE_END`.
Closes facebook/yoga#344

Reviewed By: dshahidehpour

Differential Revision: D4459950

Pulled By: emilsjolander

fbshipit-source-id: b57eb7a5b1c181a364913c3200a3794a2b7b31a6
nicktate pushed a commit to nicktate/react-native that referenced this issue Feb 9, 2017
Summary:
Followup of facebook#335, fix facebook#326. This commit add the `YGLayoutGetBorder(node, edge)` function, which correctly takes RTL/LTR into account when resolving `EDGE_START` & `EDGE_END`.
Closes facebook/yoga#344

Reviewed By: dshahidehpour

Differential Revision: D4459950

Pulled By: emilsjolander

fbshipit-source-id: b57eb7a5b1c181a364913c3200a3794a2b7b31a6
nicktate pushed a commit to nicktate/react-native that referenced this issue Feb 9, 2017
Summary:
Followup of facebook#335, fix facebook#326. This commit add the `YGLayoutGetBorder(node, edge)` function, which correctly takes RTL/LTR into account when resolving `EDGE_START` & `EDGE_END`.
Closes facebook/yoga#344

Reviewed By: dshahidehpour

Differential Revision: D4459950

Pulled By: emilsjolander

fbshipit-source-id: b57eb7a5b1c181a364913c3200a3794a2b7b31a6
nicktate pushed a commit to nicktate/react-native that referenced this issue Feb 9, 2017
Summary:
Followup of facebook#335, fix facebook#326. This commit add the `YGLayoutGetBorder(node, edge)` function, which correctly takes RTL/LTR into account when resolving `EDGE_START` & `EDGE_END`.
Closes facebook/yoga#344

Reviewed By: dshahidehpour

Differential Revision: D4459950

Pulled By: emilsjolander

fbshipit-source-id: b57eb7a5b1c181a364913c3200a3794a2b7b31a6
nicktate pushed a commit to nicktate/react-native that referenced this issue Feb 9, 2017
Summary:
Followup of facebook#335, fix facebook#326. This commit add the `YGLayoutGetBorder(node, edge)` function, which correctly takes RTL/LTR into account when resolving `EDGE_START` & `EDGE_END`.
Closes facebook/yoga#344

Reviewed By: dshahidehpour

Differential Revision: D4459950

Pulled By: emilsjolander

fbshipit-source-id: b57eb7a5b1c181a364913c3200a3794a2b7b31a6
nicktate pushed a commit to nicktate/react-native that referenced this issue Feb 9, 2017
Summary:
Followup of facebook#335, fix facebook#326. This commit add the `YGLayoutGetBorder(node, edge)` function, which correctly takes RTL/LTR into account when resolving `EDGE_START` & `EDGE_END`.
Closes facebook/yoga#344

Reviewed By: dshahidehpour

Differential Revision: D4459950

Pulled By: emilsjolander

fbshipit-source-id: b57eb7a5b1c181a364913c3200a3794a2b7b31a6
nicktate pushed a commit to nicktate/react-native that referenced this issue Feb 9, 2017
Summary:
Followup of facebook#335, fix facebook#326. This commit add the `YGLayoutGetBorder(node, edge)` function, which correctly takes RTL/LTR into account when resolving `EDGE_START` & `EDGE_END`.
Closes facebook/yoga#344

Reviewed By: dshahidehpour

Differential Revision: D4459950

Pulled By: emilsjolander

fbshipit-source-id: b57eb7a5b1c181a364913c3200a3794a2b7b31a6
nicktate pushed a commit to nicktate/react-native that referenced this issue Feb 9, 2017
Summary:
Followup of facebook#335, fix facebook#326. This commit add the `YGLayoutGetBorder(node, edge)` function, which correctly takes RTL/LTR into account when resolving `EDGE_START` & `EDGE_END`.
Closes facebook/yoga#344

Reviewed By: dshahidehpour

Differential Revision: D4459950

Pulled By: emilsjolander

fbshipit-source-id: b57eb7a5b1c181a364913c3200a3794a2b7b31a6
nicktate pushed a commit to nicktate/react-native that referenced this issue Feb 9, 2017
Summary:
Followup of facebook#335, fix facebook#326. This commit add the `YGLayoutGetBorder(node, edge)` function, which correctly takes RTL/LTR into account when resolving `EDGE_START` & `EDGE_END`.
Closes facebook/yoga#344

Reviewed By: dshahidehpour

Differential Revision: D4459950

Pulled By: emilsjolander

fbshipit-source-id: b57eb7a5b1c181a364913c3200a3794a2b7b31a6
nicktate pushed a commit to nicktate/react-native that referenced this issue Feb 9, 2017
Summary:
Followup of facebook#335, fix facebook#326. This commit add the `YGLayoutGetBorder(node, edge)` function, which correctly takes RTL/LTR into account when resolving `EDGE_START` & `EDGE_END`.
Closes facebook/yoga#344

Reviewed By: dshahidehpour

Differential Revision: D4459950

Pulled By: emilsjolander

fbshipit-source-id: b57eb7a5b1c181a364913c3200a3794a2b7b31a6
nicktate pushed a commit to nicktate/react-native that referenced this issue Feb 9, 2017
Summary:
Followup of facebook#335, fix facebook#326. This commit add the `YGLayoutGetBorder(node, edge)` function, which correctly takes RTL/LTR into account when resolving `EDGE_START` & `EDGE_END`.
Closes facebook/yoga#344

Reviewed By: dshahidehpour

Differential Revision: D4459950

Pulled By: emilsjolander

fbshipit-source-id: b57eb7a5b1c181a364913c3200a3794a2b7b31a6
nicktate pushed a commit to nicktate/react-native that referenced this issue Feb 9, 2017
Summary:
Followup of facebook#335, fix facebook#326. This commit add the `YGLayoutGetBorder(node, edge)` function, which correctly takes RTL/LTR into account when resolving `EDGE_START` & `EDGE_END`.
Closes facebook/yoga#344

Reviewed By: dshahidehpour

Differential Revision: D4459950

Pulled By: emilsjolander

fbshipit-source-id: b57eb7a5b1c181a364913c3200a3794a2b7b31a6
nicktate pushed a commit to nicktate/react-native that referenced this issue Feb 9, 2017
Summary:
Followup of facebook#335, fix facebook#326. This commit add the `YGLayoutGetBorder(node, edge)` function, which correctly takes RTL/LTR into account when resolving `EDGE_START` & `EDGE_END`.
Closes facebook/yoga#344

Reviewed By: dshahidehpour

Differential Revision: D4459950

Pulled By: emilsjolander

fbshipit-source-id: b57eb7a5b1c181a364913c3200a3794a2b7b31a6
nicktate pushed a commit to nicktate/react-native that referenced this issue Feb 9, 2017
Summary:
Followup of facebook#335, fix facebook#326. This commit add the `YGLayoutGetBorder(node, edge)` function, which correctly takes RTL/LTR into account when resolving `EDGE_START` & `EDGE_END`.
Closes facebook/yoga#344

Reviewed By: dshahidehpour

Differential Revision: D4459950

Pulled By: emilsjolander

fbshipit-source-id: b57eb7a5b1c181a364913c3200a3794a2b7b31a6
@facebook facebook locked as resolved and limited conversation to collaborators May 30, 2018
@react-native-bot react-native-bot added the Resolution: Locked This issue was locked by the bot. label Jul 23, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Resolution: Locked This issue was locked by the bot.
Projects
None yet
Development

No branches or pull requests

10 participants