[TIMOB-25883] iOS - "Callback was already called." error thrown when source contains ES6 error #9969
Conversation
| this.logger.log(); | ||
| process.exit(1); | ||
| } finally { | ||
| this.unmarkBuildDirFile(to); |
There was a problem hiding this comment.
Does it make a different to call cb2() before or after unmarking a file? It was called after it previously and is called after it now. Still in testing
There was a problem hiding this comment.
In order to surface the nicer error output, I catch the error and spit it out here and exit the process rather than propagating it up the callback stack.
the unmarking happens before the error handling.
|
hmmmm. I assumed it would work the same on both. But it does not. And I have no idea why! |
|
So, trying to dig in here, I think this is a side-effect of babel's set of plugins to use. Basically we use babel-preset-env which is supposed to narrow the list of plugins to get involved in the transpilation based on the target JS engine. In this case, I believe it is running this plugin on iOS, but not android: https://babeljs.io/docs/plugins/check-es2015-constants/ Why it would get applied to only some but not others is a bit weird to me, since this plugin just does a build-time check and there's a separate plugin to transform to var usage/etc. Maybe iOS would silently ignore the failure on older engines? In any case, this isn't something we can directly control unless wI start adding the ability to pass along plugins to add to the babel transpilation set (and then pass in this plugin on Android when transpiling). I'm hesitant to do that here since Babel 7 is likely to change the set of plugins/checks done as well. |
|
Just to confirm, they are removing this specific check in babel 7 due to some spec compliance issue (I think because technically this is a runtime error so the parser/transformer shouldn't die on it). It's possible we could try and revive a copy in our own node-titanium-sdk repo to do a build time check here, but I don't know if we ought to... |
|
@hansemannn ping |
|
@sgtcoolguy I thought we are blocked by the Babel change. This PR won't go live before 7.2.0, which is at least 1-2 months away from now. So if Babel 7 proves to be stable until then, we could use that one. |
sgtcoolguy
dismissed
hansemannn’s stale review
Due to set of plugins Babel uses based on target JS engines, android will not show same build time error - which is OK because in babel 7 it won't show it for iOS either because it is supposed to be a runtime error.
|
QE? |
JIRA: https://jira.appcelerator.org/browse/TIMOB-25883
Description:
If a JS file has syntax errors or the transpile/parse/transform process throws an Error, we didn't surface that properly. Now we wrap the calls to
jsanalyze(that does that work) intry/catchand call the wrapping callback with the error so that it gets surfaced up to the CLI.Babel is awesome and gives us pre-formatted contextual code to show the error, so we get super-nice results in this scenario:
