-
Notifications
You must be signed in to change notification settings - Fork 633
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
Function toString does not behave the same with hermes turned on #114
Comments
Hermes compiles all source to bytecode before executing it on a phone. As such, we discard source information including the actual text of the code, parameter names, etc. during compilation. This allows us to keep the size of our bytecode file small and ensure that the code is quick to execute. Due to this, we do not support actually getting the source code using |
Ahh, interesting. Thanks for the explanation! I’ll close this issue then. |
I don't suppose there'd be any way to inform Hermes that we'd like to hold onto the conventional result of a call |
@cawfree Correct: Hermes does not currently support a way to include the original source in the bytecode bundle. |
@cawfree this is an interesting idea. Perhaps we could consider a kind of annotation telling our compiler to preserve the source of a particular function. I can see how obtaining the source in a different way may become very cumbersome. I think it is doable. The question is what would such an annotation look like. One possibility would be to include "use source" in the beginning of the function (similar to "use strict"). Do you have any ideas or a preference? |
You may want to mirror https://github.com/tc39/proposal-function-implementation-hiding and go with something like “show implementation”. |
@tmikov Were you able to get somewhere in implementing this? One of our production apps broke after upgrading to hermes because we call toString() on functions and it doesn't return the expected results. |
At various points in the DFA, it seems like it would be possible to know that toString() is being called on a function object/prototype, and then only include the source for those precise functions. If that is possible, figuring how to store them in the binary format so the function string usages are friendly to CPU prefetch and don’t disrupt current L1 cache hit rates would be nice to have (tm). |
Unfortunately we don't have a use case for this internally, so the priority for implementing it is not very high. I would love to see a PR from community, if anyone is interested I can give detailed pointers about what needs to be done. |
Hi, I can work on that. Could you write down all the details? |
@lb90 sorry for the delay! Are you still interested? |
Hi @tmikov! Yes |
@lb90 great, I will start filling up the details. We will do it incrementally, it should be a fun project! |
Is there someone making some progress about this? I think this case is very important when working with webview. we will injectJavaScript to webview to execute it. If Hermes cannot support this, which means webview is kind of broken. I don't understand why Hermes team thinks it's not a big problem. |
@chj-damon what is preventing you from including the source of the function as a string? |
for example, I'm using echarts in webview. it provides an option like this: {
title: {
text: 'ECharts demo'
},
tooltip: {
show: true,
formatter: (params) => params.name + ': ' + params.value,
},
legend: {
data: ['销量']
},
xAxis: {
data: ['衬衫', '羊毛衫', '雪纺衫', '裤子', '高跟鞋', '袜子']
},
yAxis: {},
series: [
{
name: '销量',
type: 'bar',
data: [5, 20, 36, 10, 10, 20]
}
]
} you see there's a formatter function I declared in the option, when Hermes was enabled, this formatter function will be transformed to |
if |
@ljharb I think the idea may be to |
@chj-damon I see the appeal of being able to reuse the same JS function in the WebView, but there is a fundamental problem with that approach - there is no guarantee that the WebView and Hermes support the same language features. You are applying one set of Babel transforms to JS for Hermes, but the WebView has a different JS engine and it may need a different set of transforms. So, as soon as you start using it for anything more complex, differences will appear. |
Given that there's no way in the language to guarantee a function is portable (can be toStringed and re-evalled elsewhere, even in the same global environment), that seems like an inherently brittle design that needs rearchitecting. |
maybe this problem will be solved if Hermes support some signature, like 'noHermes' which defined in a function, like formatted: function(params) {
'noHermes';
return params.name;
} Hermes will ignore this function if it sees 'noHermes' signature. react-native-reanimated@2 using 'worklet' to define a function that will be running in UI thread. |
Summary: This diff introduced the notion of "source visibility" and 1. add it to ESTree FunctionLikeNode 2. compute the visibility in semantic validator according to the override rule. This is the fundation to implemetn the Stage2 proposal "Function Implementation Hiding": https://github.com/tc39/proposal-function-implementation-hiding, as well as the `'show source'` extension that Hermes is proposing to accomendate the needs of preserving source for `toString` at #114 Note that previously by computing the visibility in semantic validation phase, we can scale this to support external AST and lazy compilation properly. Reviewed By: avp Differential Revision: D26269664 fbshipit-source-id: d4eba4a78c0e41c0cd7d2320fe2a469ba7928d95
For those of you wondering, I just tried reproducing this problem on React Native 0.71.3 and it is no longer valid. My basic reproduction scenario:
This implementation logs the following to the console regardless of weather I am in debug or release modes using Hermes engine:
It even works for hot reloading when I change the implementation inside |
For me, |
@lchenfox out of curiosity: does the new bundle downloaded using |
Yes. I fixed a bug using |
@lchenfox Hermes is not intended to run from source in production, we strongly recommend against it. |
does this mean |
Yeah. Theoretically speaking, however, using |
|
In case I may make myself unclear. Here is a demo reproduced by Code snippets
Steps
I don't know how to fix this. Please help. Thanks a lot. |
I was banging my head against that why something works in debug, but not in production. I tried implementing 'show source', without luck. Still having the same problem. Any ideas why this is not working in this context
|
@Stophface the first step to identifying the problem is to look at the source that is actually passed to Hermes after being transformed by various tools in the build pipeline. |
@tmikov How can I see the source code? Plus: I thought adding |
@Stophface In order to identify whether this is a problem in Hermes, we need to be able to examine the input given to Hermes. Unfortunately we can't help you debug parts of the build pipeline that happen before Hermes. It is quite possible that the "show source" annotation is stripped before it gets to the Hermes compiler, for example by a minifier. |
As a workaround, I added a script that stringifies the needed git module. So the processed module looks like
And I can use |
Expo performs very unstable with this feature, it works 1 out of 10 times... I finally find for the first time it bundles, it does not work. And you can add a console.log to the file that uses |
tree.js uses fn.toString() in their shader library. in RN Skia and RN WebGPU we use this feature for e2e testing as well (but here we use v8 not hermes). |
@wcandillon what functionality is missing? This task should have been closed a long time ago, since Hermes supports |
@wcandillon yes, as was explained in this comment on this same issue 3 years ago, adding a "show source" directive to the body of the function causes Hermes to preserve the source. |
I use react-native-qrcode and noticed that after upgrading the React Native v0.60 and turning on hermes, the QR code was broken. We debugged the issue and found that the problem comes when the library tries to stringify this renderCanvas function and then insert it into some HTML to show in the webview.
When calling
.toString()
on the function we found that we were actually gettinginstead of getting a string of the actual function.
I realize this is probably not a very common use case, but figured I'd report it in case anyone else is seeing anything similar.
The text was updated successfully, but these errors were encountered: