Multiline String issue in Animation.js

Support
#1

I believe this is a bug in Cesium. In Animation.js (https://github.com/AnalyticalGraphicsInc/cesium/blob/master/Source/Widgets/Animation/Animation.js#L373) there is a multiline string that escapes newlines by placing a trailing backslash instead of using string concatentation, which is the valid EcmaScript way to do multiline strings.

Using the trailing backslash became an issue for us when bundling Animation.js using WebPack. Ultimately what happened in the bundle file was the animation code was indented which added a tab to the beginning of each line in the multiline string and had the side effect of creating extra textNodes in the DOM. That changed the childNode indices on lines 383 - 390 in the code referenced above and ultimately caused a Developer Error.

Using the preferred string concatenation would fix the issue. I'd be happy to create a pull request.

#2

Your assertion that a trailing backslash is invalid JavaScript is incorrect. This approach is used throughout Cesium and is part of the ECMAScript 5 specification specifically section 7.8.4, which you can read here: http://es5.github.io/#x7.8.4

Since it’s valid ES5, this sounds like a WebPack bug to me. That being said, I’m not completely against potentially changing this in Cesium, but first I would like to here the WebPack teams response to the issue and I think there would have to be a very good reason other than that their tool doesn’t handle it. Have you raised the issue with them yet? Do you have a link?

#3

Alternatively, it would be a reasonable workaround to make the code in Animation less fragile by using querySelector instead of childNodes to locate each element.

#4

Matthew, it does look like I was misinformed about the backslash being invalid in ES5. It’s valid, but not recommended. Now that I know it is valid I will take the issue to the WebPack team and post back here with their reason of not handling the trailing backslash.

Having said that, I agree with Scott’s comment that there are less fragile ways of doing the same thing.

I’ll report back with more info,

Charlie

#5

https://github.com/webpack/webpack/issues/1161

They confirmed it was a WebPack bug. By default WebPack prepends every line in the bundled source with a tab. This can be disabled by setting output.sourceprefix equal to an empty string which is a workaround until they can fix the bug.

#6

Thanks for the followup. Was this setting the only thing needed to get Cesium to work with WebPack?

#7

No. We are using terriajs-cesium that Kevin Ring provided. https://groups.google.com/forum/#!msg/cesium-dev/fH3oiHZnHgE/31MiuygDCHoJ

Repeating what was mentioned in the link, we had to copy the CSS and image assets to a public directory and set the base URL in code. Then we had to disable WebPack’s unknownContextCritical errors in order to clean up the build output.

There’s still one remaining item that I have yet to fix which is when I use WebPack to minify the bundled output I can see there is clearly still some Cesium code that isn’t getting minified (lots of whitespace and comments even though they should be removed). I haven’t yet dug into why some of the Cesium library gets minified and some of it doesn’t.

Just so it’s all in one spot, we had to lastly set output.sourceprefix = ‘’ in the webpack.config.js file.

#8

Thanks for the summary. Assuming the same steps would be needed for a pure Cesium app, I think this would be good information for us to include in a tutorial or blog post. I’ll make a note of it for the website.

#9

Matt, FYI, terriajs-cesium is pure Cesium, just modified slightly to work with the NPM / Browserify / WebPack ecosystem.

It has a few other small changes as well that are useful in my TerriaJS / National Map work, but I routinely push those back into core Cesium.

#10

Thanks, I was under the impression it was a Cesium/Leaflet hybrid like National Map.

#11

Yep, you’re thinking of TerriaJS. TerriaJS depends on Cesium itself via the terriajs-cesium NPM package. The name isn’t great, but cesium and cesiumjs were already taken on npm.