[Packager] this["require"]() form is not supported. Needed for scala-js

[gzm0]

Consider the following two pieces of codeé

this.React = this["require"]("react-native");

this.React = this.require("react-native");

After bundling, we have the following diff:

< __d('AwesomeProject/index.ios',[],function(global, require, requireDynamic, requireLazy, module, exports) {  /**

---
> __d('AwesomeProject/index.ios',["react-native/Libraries/react-native/react-native"],function(global, require, requireDynamic, requireLazy, module, exports) {  /**
943c943
< var React = this["require"]('react-native');

---
> var React = this.require('react-native/Libraries/react-native/react-native');

In the bracket select case, the JSX processor seems to be unable to detect the inclusion, even though the two pieces of code are equivalent from a JavaScript point of view.

See the original post on SO by @ChrisJamesC.

[vjeux]

Note that browserify has the same shortcoming. The dependency analyzer just scans for require(string litteral)

[gzm0]

So are you saying that this is actually a browserify issue (as in you are using browserify internally), or just that you are emulating the same behavior?

How desirable and how hard would allowing both syntaxes be to you guys? The issue we are facing is with code that is supports the Google Closure Compiler's advanced mode (generated by Scala.js) and must bracket select in this position if we don't want the call to be renamed. On the other hand, fully minimizing the file is not an option in the development cycle in the long run as it takes too long.

[ide]

could you tell Closure that "require" is an extern? https://developers.google.com/closure/compiler/docs/api-tutorial3#externs

[gzm0]

Yes, we could. However, the Scala.js compiler is (currently) not able to emit dot selects for anything but its internal code. Have a look at scala-js/scala-js#761 for a discussion on why supporting both is hard.

[amasad]

I think we can handle this case in the RegExp. If you want to take a shot at it look at
https://github.com/facebook/react-native/blob/master/packager/react-packager/src/DependencyResolver/haste/DependencyGraph/index.js#L603
https://github.com/facebook/react-native/blob/master/packager/react-packager/src/DependencyResolver/haste/index.js#L28

Note there is another PR trying to make it so we share the same Regexp and fixes some issues with whitespace: #316

[chandu0101]

any progress on this ...

[ghost]

Thank you for reporting this issue and appreciate your patience. We've notified the core team for an update on this issue. We're looking for a response within the next 30 days or the issue may be closed.

[gzm0]

What can we do to prevent closure of the issue? This is an issue for (potentially) many JS processors, just closing it because it has been inactive seems odd...

[vjeux]

@gzmo: that's a very good point, we're working on the bot right now and we still haven't figured out all the use cases. We definitely don't want to close this issue

[amasad]

I want to experiment with moving to an AST-based require analysis, provided it doesn't regress perf that much. Especially know that @martinbigio added caching for module dependencies.

[chandu0101]

@amasad thanks for taking time to look into this, any luck ? , debugging production mode code in development phase is not fun :(

[mariussoutier]

Bump...

[vjeux]

@mariussoutier, @gzm0, @chandu0101 any one of you want to take a stab at implementing it on React Native? It is not very hard.

You need to update this regex in order to support both require(...) and ["require"](...): https://github.com/facebook/react-native/blob/master/packager/react-packager/src/DependencyResolver/lib/replacePatterns.js#L14

[mariussoutier]

Done, see #4830.

[chandu0101]

@vjeux thanks alot for pointer

@mariussoutier excellent 👍

[jsierles]

Per the discussion in #2217, this issue seems to have a solution outside of React Native. Internal solutions were downvoted in #4830 as well. So, closing this out. Feel free to discuss mre on the closed issue!