ES6 at Last

[mramato]

This PR contains all of the actual code changes to make ES6 work. In order to make this change actually reviewable, the es6-staging branch has all of the boilerplate changes that affect every Sandcastle demo and requirejs module. That leaves only ~115 files left for review, many of which are trivial changes or renames.

Here's a high level summary of all the areas that changed:

What does ES6 modules mean for internal Cesium development?

At a high level:

requirejs is gone, define is gone. You now import code via the import directive. i.e.

import Cartesian3 from './Cartesian3.js';

Instead of returning your object at the end of the file, you use export. Cesium uses default exports for it's internal modules i.e.

export default Cartesian3;

use strict is gone, modules are always strict.

While we use ES6 modules, Cesium still forbids the use of ES6 features, such as "new Promise", async/await/etc..

Other good stuff: Obviously this means way less boilerplate code, but how much? We've cut over ~25k lines of code for our code base! No more giant define blocks at the top of the file, no more wasting 20 minutes because your variables were out of order either! The size of Cesium's npm and release packages have also dropped by ~9MB, and I think we can get them even smaller.

eslint

There are a handful of new .eslintrc.json files, mostly to identify the files that are still AMD modules (Sandcastle/Workers). These are needed because you can't change the parser type with a comment directive (since the parser is the thing reading the file). We can finally detect unusued modules! So those have all been cleaned up as well.

requirejs -> rollup & clean-css

requirejs, almond, and karma-requirejs have all been removed. We now use rollup for building and minifying (via uglify) JS code and clean-css for css. These changes are fairly straight-forward and just involve calling rollup instead of requirejs in the build process.

Overall build time is significantly faster. CI is ~11 minutes compared to ~17 in master. Running makeZipFile on my machine takes 69 seconds compared to 112 seconds in master. There's probably plenty of room for additional optimization here too.

We wrote an published a small npm module, rollup-plugin-strip-pragma, for stripping the requirejs pragmas we use out of the release builds. This is maintained in the Tools/rollup-plugin-strip-pragma directory.

As for what we produce. The built version of Cesium is now a UMD module. So it should work anywhere that hasn't made the jump to ES6 yet. For users that were already using the "legacy" combined/minified approach, nothing changes.

One awesome thing about roll-up is that it compiles all of the workers at once and automatically detects shared codes and generates separate bundles under the hood. This means the size of our worker modules shrink dramatically and Cesium itself will load them much faster. The total minified/gzipped size of all workers in master is 2.6 MB compared to 225 KB in this branch! This should be most noticeable on demos like Geometry & Appearances which load lots of workers for the various geometry typs.

roll-up is also used to build Cesium Viewer, which is now an ES6 app.

We use clean-css via gulp and it is also a straightforward change from requirejs that requires no special mention.

Workers

While the spec allows for ES6 Web Workers, no browser actually supports them yet. That means we needed a way to get our workers into non-ES6 form. Thankfully, roll-up can generate AMD modules, which means we now have a build step to compile our Worker source code back into AMD and use the existing TaskProcessor to load and execute them. This build step is part of the standard build task and is called createWorkers. During development, these "built" workers are un-optimized so you can still debug them and read the code.

Since there is a build step, that means if you are changing code that affects a worker, you need to re-run build, or you can use the build-watch task to do it automatically.

The ES6 versions of Worker code has moved into Source/WorkersES6 and we build the workers into their "old home" of Source/Workers. cesiumWorkerBootstrapper and transferTypedArrayTest which were already non-AMD ES5 scripts remain living in the Workers directory.

Surprisingly little was changed about TaskProcessor or the worker system in general, especially considering that I thought this would be one of the major hurdles.

ThirdParty

A lot of our ThirdParty either already had a hand-written wrapper for AMD (which I updated to ES6) or had UMD which created problems when importing the same code in both Node and the browser. I basically had to update the wrapper of every third-party library to fix these problems. In some cases I updated the library version itself (Autolinker, topojson). Nothing to be too concerned about, but future clean-up would be using npm versions of these libraries and auto-generating the wrappers as needed so we don't hand-edit things.

Sandcastle

Sandcastle is eternal and manages to live another day in it's ancient requirejs/dojo 1.x form. Sandcastle now automatically uses the ES6 version of Cesium if it is available and fallsback to the ES5 unminified version if it is now. The built version of Sandcastle always uses CesiumUnminified, just like master. This means Sandcastle still works in IE11 if you run the combine step first (or use the relase zip)

• Removed Cesium usage from Sandcastle proper, since it wasn't really needed
• Generate a VERSION propertyin the gallery index since Cesium is no longer being included.
• Remove requirejs from Sandcastle bucket
• Update bucket to use the built version of Cesium if it is available by fallbackto the ES6 version during development.
• Standalone.html was also updated

There's a bit of room for further clean-up here, but I think this gets us into master. I did not rename bucket-requirejs.html because I'm pretty sure it would break previously shared demos. We can put in some backwards compatible code later on if we want. (But I'd rather just see a full Sandcastle rewrite).

Specs

Specs are now all ES6, except for TestWorkers, which remain standard JS worker modules. This means you can no longer run the unbuilt unit tests in IE11. No changes for Chrome and Firefox.

Since the specs use ES6 modules and built Cesium is an ES5 UMD, I added a build-specs build step which generates a combined ES5 version of the specs which rely on Cesium as a global variable. We then inject these files into jasmine instead of the standard specs and everything works exactly as it did before. SpecRunner.html has been updated to inject the correct version of the script depending on the build/release query parameters.

The Specs must always use Cesium by importing Source/Cesium.js, this is so we can replace it with the built Cesium as describe above.

There's a bunch of room for clean-up here, such as unifying our two copies of jasmine into a single helper file, but I didn't want to start doing that clean-up as part of this already overly big PR. The important thing is that we can still test the built version and still test on IE/Edge as needed.

I also found and fixed two bugs that were causing failing unit tests, one in BingMapsImageryProviderSpec.js (which was overwriting createImage andnot setting it back) and ShadowVolumeAppearance.js (which had a module level caching bug). I think these may have been the cause of random CI failures in master as well, but only time will tell.

For coverage, we had to switch to karma-coverage-istanbul-instrumenter for native ES6 support, but that's it.

Finally, I updated appveryor to build Cesium and run the built tests under IE. We still don't fail the build for IE, but we should probably fix that if we want to keep it going.

NodeJS

When NODE_ENV is production, we now require in the minified CesiumJS directly, which works great because it's now a UMD module. Otherwise, we use the excellant esmpackage to load individual modules, it was a fairly straightforward swap from our old requirejs usage. We could probably drop esm too if we don't care about debugging or if we provie source maps at some point.

What's next?

I tried to change as little formatting as possible to make a code review possible, however we now have uneeded indentation throughout our code. We should look into doing a code-wide autoformat, probably using eslint rules or one of the standard tools that work with it (this would also make it easy to maintain consistent formatting going forward). CC #5369

We should now be able to provide source maps for our users to make debugging with the minified JS code possible (not to mention make our lives easier). I'll write up an issue

Should update https://cesium.com/docs/tutorials/cesium-and-webpack/ or perhaps it is so trivial it's no longer needed.

Allow ES6/7 features: We should be able to keep our minimal build step policy and still use newer features like async/await/Promise/etc.. This is because Chrome/Firefox/Safari all support these features natively. We would then only run the final output through Babel as part of the combine/minifyRelease processes. This would make our code a lot nicer with almost no overhead during day to day development (but more research is needed).

I need to review our developer documentation and make any minor updates so it's not wrong. Ultimately, I realized while doing this that we need better infrastructure documentation over all.

And of course, lots of clean-up possible, I mentioned some of it above but I'll write separate issues as I go through my notes.

I'm sure I left out some details and I'm sure there were some next steps that I meant to write down and have since forgot, but I think the above should give everyone a fighting chance to review this PR.

I'm going to continue self-reviewing everything but this is ready for final review from whomever is wiling to take the time to look.

Fixes #8122

[cesium-concierge]

Thanks for the pull request @mramato!

• ✔️ Signed CLA found.
• ❔ Changes to third party files were made.
  • Looks like a file in one of our ThirdParty folders (ThirdParty/, Source/ThirdParty/) has been added or modified. Please verify that it has a section in LICENSE.md and that its license information is up to date with this new version.
• ❔ Unit tests were not updated.
  • Make sure you've updated tests to reflect your changes, added tests for any new code, and ran the code coverage tool.

Reviewers, don't forget to make sure that:

• Cesium Viewer works.
• Works in 2D/CV.
• Works (or fails gracefully) in IE11.

[mramato]

Terria is a mix of CommonJS and ES6 modules at the moment, so I did have to change all the requires in the CommonJS modules from require('whatever/cesium/module') to require('whatever/cesium/module').default. I guess maybe esm papers over that difference in a proper node context?

@kring This sounds more like a quirk of whatever loader you are using more than anything else. What system are you using? I wonder if switching to named modules would have any affect or if it would simple require require('whatever/cesium/module').Cartesian3 instead (which would actually be worse in my opinion).

[markerikson]

@mramato , @kring : that's pretty standard behavior any time you're mixing import behavior between different module formats (CommonJS, AMD, ESM). Or at least, it's the way Webpack (and presumably other loaders) implement it.

[mramato]

Now that 1.61 is out, I would like to get this merged up to master ASAP. So if anyone has any critical feedback or strong opinions about changes that affect the end user, please let us know sooner rather than later. Once this is in master we will absolutely continue to improve it, I'm just trying to avoid any more huge sweeping changes.

@OmarShehata, thanks for volunteering to do a formal review here. Can that happen soon? I really wanted to this in master like today, but by the end of the week is probably more realistic.

[TJKoury]

I am using this branch in my celestrak.com build; no issues to report.

[mramato]

Awesome, thanks @TJKoury!

[mramato]

While doing some self review, I realized that Cesium.js has some subtle difference (which don't matter in practice) from master.

• Shaders are put at the top level instead of a sub-object, but they were always private to begin with. So instead of Cesium._shaders.AcesTonemappingStage it's not Cesium._shadersAcesTonemappingStage I think this is fine since they are still private and it makes the code similar.
• Third party libraries that had special characters in names, such as Cesium['knockout-3.5.0'] are now exported as Cesium.knockout_3_5_0 This is because ES6 restricts export names to valid identifies.

None of these issues are part of the official API, just something I noticed and wanted to bring up.

Currently the CombinedUnminified version in this branch is bigger than master and I have no idea why (it's significantly less lines of code) Trying to figure out the difference.

[OmarShehata]

I'm planning on taking a look and doing some testing tomorrow.

[TJKoury]

@mramato I ran into the ._shaders issue as well; in my working copy I had put all those in a _shaders object that was exported, but since nothing in Cesium uses those in that nomenclature as you mentioned.

Happy to continue testing this branch in production. Thanks again for cranking this out so quick.

[OmarShehata]

Reviewing this was not as bad as I thought it'd be! The only broken thing I found was that the Sandcastle standalone doesn't seem to work. It's loading Cesium.js from the right URL, but all the other asset files from the wrong URL and they 404.

Some other thoughts:

• You mentioned writing an issue to generate source maps to get better stack traces for minified Cesium. Did you write that up? If not please do, that'll help a lot when debugging people's issues.
• The build CesiumViewer is almost twice as small! 17 mb to 10 mb.
• Should we add "This file is generated" to the Workers/* just like we do with shaders? I can imagine a developer accidentally modifying that file instead of WorkersES6.
• I am in favor of squashing the commit history, mostly because a commit should be a functional version of the library, and if this just has a lot of commits mid-transition I don't think we need to keep that around.
• So ESM is a legit npm dependency of Cesium that gets bundled and ships with the library? So this opens up a path to getting rid of ThirdParty/ and having those all as npm modules? If so, can you write up an issue for that?
• I am leaning towards not using export default, just because of this explicit naming philosophy, that you know for sure no one can do import blah as 'Cartesian3' and confusing others on the team, but I don't know if this has any adverse consequences.
• I haven't yet tried how much easier this makes running Cesium in a new webpack/parcel project, but I want to try that and report here.

[mramato]

• You mentioned writing an issue to generate source maps to get better stack traces for minified Cesium. Did you write that up? If not please do, that'll help a lot when debugging people's issues.

#8245

• Should we add "This file is generated" to the Workers/* just like we do with shaders? I can imagine a developer accidentally modifying that file instead of WorkersES6.

Not sure how helpful it will be, but it was easy enough to add so I did it.

• So ESM is a legit npm dependency of Cesium that gets bundled and ships with the library? So this opens up a path to getting rid of ThirdParty/ and having those all as npm modules? If so, can you write up an issue for that?

No, ESM is just how we load the unbuilt version of Cesium into Node.JS which is makes node-based development easier to debug. It's not even required if we just always wanted to use the combined version there. It is not bundled or shipped with anything. This is no different than how we used to depend on requirejs on the node side.

Switching to npm dependencies for Cesium's Source/ThirdParty is not trivial or something we will probably do anytime soon. The fact that we need to support both Node and the Browser as well as shim some of the libraries makes it tough. There's also stuff in Source/ThirdParty that isn't even on npm.

• I am leaning towards not using export default, just because of this explicit naming philosophy, that you know for sure no one can do import blah as 'Cartesian3' and confusing others on the team, but I don't know if this has any adverse consequences.

Nothing stops people from doing this even with named exports, the syntax is just slightly different. In fact, if we did this then everything we import CesiumMath would be a special case because it would be Math as CesiumMath. If someone can provide a good technical reason for avoid default exports, I'm all ears, but so far it seems like an opinion thing and it's just not worth the time to do the change. I'm not even against it, it is just another sweeping change that doesn't seem to have rhyme or reason (or time investment) to make.

• I haven't yet tried how much easier this makes running Cesium in a new webpack/parcel project, but I want to try that and report here.

Any changes to https://cesium.com/docs/tutorials/cesium-and-webpack/ and a PR into https://github.com/AnalyticalGraphicsInc/cesium-webpack-example would be greatly appreciated.

[TJKoury]

@mramato @OmarShehata Not trying to be pushy, but as I mentioned above adding "type":"module" in the package.json allows all of the converted modules in the subfolder to be used natively in node with the --experimental-modules flag.

No need for esm, no side effects.

[mramato]

@TJKoury it's on my list, no worries.

[OmarShehata]

I think this is all 👍 from me. @mramato if you have time to look into why standalone mode isn't working here that'd be great, but I'm also willing to merge this and have a priority-next-release issue to fix that. That way it can get more testing in master sooner than later.

[mramato]

@OmarShehata I'm debugging the standalone thing now. It only affects built sandcastle, but could be the sign of a larger issue when trying to auto-detect CESIUM_BASE_URL. (also have some other changes from your review to push). I'll bump when ready

[mramato]

@TJKoury I did an initial attempt of adding `"type": "module" to package.json, but it didn't work. If you can write up an issue (or open a PR) with more context we'll happily take a look at it in time for Nov 1. We just need any solution that supports Node 10 and 12 both ES6 and commonjs.

[mramato]

@OmarShehata I believe I addressed all of your issues or otherwise replied to your feedback. Thanks for the review and catching a few things I missed.

This should be ready.

[TJKoury]

@mramato make sure to use node --experimental-modules to load ES6 modules.

[TJKoury]

If you still are having issues I'll open an issue.

[mramato]

If you still are having issues I'll open an issue.

Yes, please do. I haven't done anything with ES6 modules in Node yet so I'm sure it's something simple I'm missing.

[OmarShehata]

🤞

[mramato]

There's now a blog with more details about this transition up on https://cesium.com/blog/2019/10/31/cesiumjs-es6/