When composing source maps, utilize source content provided in an input map #2921
Conversation
ChadKillingsworth
force-pushed
the
source-maps-content
branch
4 times, most recently
from
d6d4290
to
c5d62da
Compare
ChadKillingsworth
force-pushed
the
source-maps-content
branch
from
c5d62da
to
4fc4b73
Compare
|
|
||
| private static String resolveSibling(String path1, String path2) { | ||
| List<String> path1Parts = new ArrayList(Arrays.asList(path1.split("/"))); | ||
| List<String> path2Parts = new ArrayList(Arrays.asList(path2.split("/"))); |
There was a problem hiding this comment.
Let's add diamond operators.
| Compiler compiler = new Compiler(); | ||
| Result result = compiler.compile(externs, ImmutableList.copyOf(sources), options); | ||
| assertTrue(result.success); | ||
| assertEquals(result.errors.length, 0); |
There was a problem hiding this comment.
I think we get a little better error message if we reword as:
assertThat(result.errors).isEmpty();
| Compiler compiler = new Compiler(); | ||
| Result result = compiler.compile(externs, ImmutableList.copyOf(sources), options); | ||
| assertTrue(result.success); | ||
| assertEquals(result.errors.length, 0); |
| path2Parts.remove(path2Pos); | ||
| } else if (path2Parts.get(path2Pos).equals("..")) { | ||
| path2Parts.remove(path2Pos); | ||
| path1Parts.remove(path1Parts.size() - 1); |
There was a problem hiding this comment.
I'm seeing crashes on this loop like:
java.lang.IndexOutOfBoundsException: Index -1 out-of-bounds for length 0
at java.base/jdk.internal.util.Preconditions.outOfBounds(Preconditions.java:64)
at java.base/jdk.internal.util.Preconditions.outOfBoundsCheckIndex(Preconditions.java:70)
at java.base/jdk.internal.util.Preconditions.checkIndex(Preconditions.java:248)
at java.base/java.util.Objects.checkIndex(Objects.java:372)
at java.base/java.util.ArrayList.remove(ArrayList.java:517)
at com.google.javascript.jscomp.Compiler.resolveSibling(Compiler.java:3730)
at com.google.javascript.jscomp.Compiler.getSourceMapping(Compiler.java:2931)
at com.google.javascript.jscomp.LightweightMessageFormatter.format(LightweightMessageFormatter.java:96)
at com.google.javascript.jscomp.LightweightMessageFormatter.formatWarning(LightweightMessageFormatter.java:80)
at com.google.javascript.jscomp.JSError.format(JSError.java:174)
|
Fixed. Let me know if you want me to squash the commits. |
|
LGTM, let's squash and I'll bring it in! Thanks Chad |
|
Hmm 2008247 did some of the same work. I'll have to see what of my changes still needs applied. |
ChadKillingsworth
force-pushed
the
source-maps-content
branch
from
1852533
to
b44676a
Compare
…ce file cannot be located
ChadKillingsworth
force-pushed
the
source-maps-content
branch
from
b44676a
to
9596c0e
Compare
|
OK, looks good. Was dropping the unit tests intentional? |
|
@blickly I prefer to have the tests, but they were not actually failing (at least when I ran it). I'm not sure why. A lot of things changed with the other commit and I haven't had a chance to fully understand the implications. |
…ce file cannot be located Closes google#2921 ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=197595899
I discovered that with the JS version, source maps are not composed. This turned out to be from a bug where we only use the sourcesContent from an input source map to display errors and we only load the content from disk.
In addition, we only composed source maps if the content of the original file could be loaded. This wasn't needed.
Adds support to source maps where content provided in an input source map can be forwarded to the output source map. Also allows the GWT version to compose source maps when the input source map provides the original content in the
sourcesContentfield.