Bad generation of source-maps in libraries for @extern classes

[carlosrovira]

Hi,

in Jewel library we have a dialogPolyfill class that is marked wih @externs
it should be used in Alert.as

`dialogPolyfill.registerDialog(dialog);`

but we have for now this:

`window["dialogPolyfill"]["registerDialog"](dialog);`

since compiler has a bug when generating source-maps for an @externs class that's located in a library (seems this not happen if the extern class is in the app, see hljs.as in tourdejewel for and example)

Using the class dialogPolyfill we get in release mode this error:

end of list of source files
may 09, 2019 11:50:28 PM com.google.javascript.jscomp.LoggerErrorManager println
GRAVE: externs/dialogPolyfill.js.map:2: ERROR - Parse error. Semi-colon expected
"version":3,

[joshtynjala]

@carlosrovira Okay, I'll try to take a look next week. My initial thought is that we don't need to generate source maps at all for externs. Since externs don't result in real JS, the source maps won't actually be used by the debugger.

[carlosrovira]

@joshtynjala, it seems right to me, so I can try to do it. Can you point me to the file where I should look?
If I see I finally can do myself I'll report back to you

[joshtynjala]

Here is where the source map file is being written:

https://github.com/apache/royale-compiler/blob/develop/compiler-jx/src/main/java/org/apache/royale/compiler/internal/codegen/js/JSWriter.java

I see an isExterns variable that you might be able to use to skip the source map creation.

Additionally, in one of the emitters, it adds a special JS comment that browsers look for to determine if there's a source map. I don't think it will break anything if this comment points to a file that doesn't exist. However, if I'm wrong, that would need to be skipped for externs too.

[joshtynjala]

I took a look at this issue today, and here are my findings.

GRAVE: externs/dialogPolyfill.js.map:2: ERROR - Parse error. Semi-colon expected

Unfortunately, I am not able to reproduce this error message.

It seems to indicate that there's something wrong with the .js.map file. That's interesting because we don't actually write that file. Classes from the Google Closure Compiler handle it for us. It's strange that it works correctly for me, but not for you.

It will be difficult to fix this issue if I cannot reproduce.

since compiler has a bug when generating source-maps for an @externs class that's located in a library (seems this not happen if the extern class is in the app, see hljs.as in tourdejewel for and example)

I can reproduce this issue. If the externs were generated from a .as file and are compiled into a .swc file that is referenced by an app project, they seem to be ignored. On the other hand, if they are generated from an .as file, but they're on the app project's source path, they work correctly, and renaming does not happen.

I have touched the code related externs here and there, but I know other parts of the compiler better. I would probably need to spend several hours investigating to figure out more. The best that I can say right now is that sometimes externs are ignored when they come from a .swc file, but I don't know exactly why. Especially since we include several .swc files in Royale that contain externs, and I haven't heard about any issues where those fail to work correctly.

After looking at both issues, I do not think that they are related. Even if source maps were not generated for externs, you'd still run into this renaming issue, I think.

[carlosrovira]

Hi Josh,

Unfortunately, I am not able to reproduce this error message.

sorry about my late response, there was holidays here in Spain. To get the error you must to switch comments in Jewel Alert.as lines 225 and 226. In repository right now is using bracketed access and changing that will use the externs class with dot notation and the .js.map.

For the second problem, about locating externs in .swc and in app I think it will be solved removing the generation of the .js.map. Maybe I could be wrong. Some days ago I could make the compiler not generate the .js.map code, but I could not avoid the creation of the .js.map file itself. I'm going to try it to see if I can reach the point where that file is created. Hope to give some update later this day.

Thanks for taking a look Josh! :)

[joshtynjala]

For the second problem, about locating externs in .swc and in app I think it will be solved removing the generation of the .js.map.

I was able to prevent the creation of the .js.map file in the .swc, but it made no difference. In the app project, the method was still renamed by Closure.

[carlosrovira]

Hi Josh, are you saying that you get problem one solved but second is still there? Please let me know. Thanks!

[joshtynjala]

I'm saying that preventing the .js.map file from being generated did not solve any issues for me.

[carlosrovira]

Can you commit that change so .js.map will be prevented from being generated for externs?
Thanks

[joshtynjala]

I did not keep my changes because they did not seem to fix anything.

[carlosrovira]

@joshtynjala I finally solved the problem, but although now it works, it could be improved making the sourcemap file for @extern not created. Right now the fix is just removing the source map directive in the @externs js. Now I'll try to do make that file not to be generated.