Plan to switch to ES6 syntax

[ebogo1]

Now that IE support has been dropped in 1.84, we should make a plan to update our code. This issue can be a place to brainstorm ideas for these changes.

There are some things that need to happen before we can start making changes in CesiumJS:

• Update coding guide
• Update eslint config

Other things to think about:

• The when library is not completely compatible with native promises, use of async and await might need to be delayed until we get rid of when.
• Some ES6 features might be a performance downgrade from ES5
• Can we make the switch from var to let and const now? Some of this effort already started with the last gltf-pipeline PR: Update build-cesium task to account for prettier formatting gltf-pipeline#600.

[ptrgags]

I'm curious about the switch to let and const, is this going to be a change for new code moving forward, or are you translating all the old code to use let and const? (are there tools for this? sounds like a big task otherwise)

I'm also curious to see when we can use for ... of loops for objects instead of the rather verbose:

for (var key in obj) {
    if (obj.hasOwnProperty(key)) {
    }
}

And other ES6 features (e.g. spread syntax where right now we use shallow combine()... there's a lot of helpful things to consider here (of course, weighing this against any performance differences)

[wewindy]

I'm glad to see the official intention to update the syntax of the source code.

Using let and const avoids the problem of variable promotion in JavaScript:

if (isUnderground) { // <-  Error, `isUnderground` is not defined
  // do something
}

let isUnderground = false

if use var the above code will run well:

if (isUnderground) {
  // do something
}

var isUnderground = false

const maintains the immutability of variables:

import { Cartesian3 } from 'cesium'

const coords = new Cartesian3(100, 200, 0)
coords = 0 // Uncaught TypeError: Assignment to constant variable.

If you use var to make the code run normally, there is generally no need to change it to let and const.

For the for ... of mentioned by @ptrgags , I have seen in some evaluation reports that the performance of iterative access to the object's key and value is acceptable, but the performance of iterative access to the array may not be as good as that of for ( let i = 0; i <someLength; i++) {/* some code */ } and the Array.prototype.forEach method.

Modularization technology has been changed from requirejs to esmodule in version 1.63, which has done a good job.

I have used the vitejs packaging tool, and have used CesiumJS in the reactjs and vuejs frameworks. Adding some guidance documents combined with these front-end frameworks maybe a good idea.

The above are some of my simple views, I hope it will be useful to the official.

---

In addition, is it possible to extract the math module of CesiumJS (most of the codes is in the Source/Core directory) separately into an npm package? This is a relatively complete js math library I have seen so far.

[ebogo1]

Related issue: #9610

[ptrgags]

First of all, I just want to say I'm very excited to see that #10026 got merged yesterday! Already enjoying being able to use const/let in my code

I'm very curious about what's next, as there's many ES6+ features I wish I could be using. this site has a good overview of features, though it's only ES6, not the later versions (e.g. async/await is actually ES2017). Of course, I realize that some of these features (in particular loops) need to be considered with care in regards to performance.

Given what my team is working on (3D Tiles Next features and ModelExperimental), these are some ES6 (ES2015) syntax that I would find very helpful day-to-day for more readable code:

Small Improvements

These features would be nice to have, at least for new code.

• String template literals. I'm doing a lot of shader generation, and this would look nicer than concatenating string parts with +. Plus, it allows multi-line literals which would simplify some sandcastle examples
• the ES6 Set class would be helpful for CustomShader, right now I'm using a dictionary as a substitute.
• fat arrow notation. (x) => {} is a lot nicer than `function(x) {} for anonymous functions, and it handles "this" better.
• Property shorthand

// I want to do this
return {
  property1,
  property2
}

// instead of this which I find myself doing quite often in ES5
return {
  property1: property1,
  property2: property2
}

• destructuring assignment

const [x, y, z] = packedPosition;

// instead of
const x = packedPosition[0];
const y = packedPosition[1];
const z = packedPosition[2];

Larger Questions:

• Upgrading old CesiumJS code is a massive task. Do we always need to update everything at once (like the const/let changes) or can we do it gradually (allow new syntax for new files, update old code in smaller chunks?). I can imagine the latter would make the codebase more fragmented style-wise, but it would at least allow better quality-of-life for new development.
• for-of loops is perhaps the feature I want the most (especially for iterating over dictionaries). However, if performance is an issue, then we should be careful here.
  • We should do some benchmarks ourselves to see how big of a difference it is
  • It's also important to make a distinction of where the loop appears. There's a big difference between one-time initialization code and code that executes every frame. Best practices should be documented in the Coding Guide, but they would not be easily enforceable by eslint.
• Are there any CesiumJS built-in functions we can phase out in favor of ES6+ syntax? (some of these are longer-term wishes)
  • combine(b, a, deep=false) could be written {...a, ...b}.
  • Likewise clone(a, deep=false) could be written {...a}
  • (ES2020 only) defaultValue(value, default) could someday be written value ?? default (see nullish coalescing operator on MDN
  • (ES2020 only) optional chaining would be nice to reduce the number of defined() checks. E.g. const metadata = tilesetJson?.extensions.?3DTILES_metadata;
  • anything else I'm forgetting?
• Switching from when -> native Promise would be nice (though there are some subtleties, see Replace when with native Promises #8525). Even better would be async/await, but that's only available in ES2017+
• Longer term, would we be able to start using class syntax, at least for new classes? the syntax is much nicer (especially for getters/setters).

[ggetz]

As a part of this, we should revisit Cesium's default eslintrc configuration as a whole (and the shared eslint-cesium plugin in this repository) to remove the out of date and unneeded rules keeping us on es5 syntax.

[sanjeetsuhag]

Background

• Look into eslint-plugin-compat eslint plug-in #7102
  • Close since we don't support IE11 anymore.
• Investigate why guard-for-in doesn't work for all cases #8100
  • Investigate then close

Motivation

CesiumJS currently disallows using many language features of ES6 (as seen here). As part of our efforts to modernize the CesiumJS code base and tooling, we want to revisit our usage of ESLint and take advantage of the latest features. This will be beneficial to all contributors to CesiumJS.

Proposed Changes

Move eslint-config-cesium to its own repository

• ESLint allows for creation of shareable configurations to allow for consistency between projects, and within the CesiumJS repo, we maintain the eslint-config-cesium configuration. It is used across several repositories, albeit not all of them are updated to the latest version.
• There are three configurations exported:

  graph TD;
	  eslint:recommended-->eslint-config-cesium:::cesium;
      eslint-config-cesium-->eslint-config-cesium/browser:::cesium;
      eslint-config-cesium-->eslint-config-cesium/node:::cesium;
  classDef cesium color:#6dabe4

Loading

Upgrading eslint configuration in CesiumJS

• Inside CesiumJS, there are 6 configurations:

  • .eslintrc.json
  • Apps/.eslintrc.json
  • Apps/Sandcastle/.eslintrc.json
  • Apps/TimelimeDemo/.eslintrc.json
  • Source/.eslintrc.json
  • Specs/.eslintrc.json
  • Specs/TestWorkers/.eslintrc.json

• Here's how they depend on each other, with ES6 configurations in green and ES5 configurations in red:

  graph TD;
	  eslint-config-cesium/node.js:::ES6-->.eslintrc.json:::ES6;
	  eslint-config-prettier:::ES6-->.eslintrc.json:::ES6;
	  eslint-config-cesium/browser.js:::ES6-->Source/.eslintrc.json:::ES5;
	  eslint-config-prettier-->Source/.eslintrc.json;
	  eslint-plugin-es:::ES5-->Source/.eslintrc.json;
	  Source/.eslintrc.json:::ES5-->Apps/.eslintrc.json;
	  Apps/.eslintrc.json:::ES5-->Apps/TimelineDemo/.eslintrc.json:::ES5;
	  Apps/.eslintrc.json-->Apps/Sandcastle/.eslintrc.json:::ES5;
	  Source/.eslintrc.json-->Specs/.eslintrc.json:::ES5;
	  Specs/.eslintrc.json-->Specs/TestWorkers/.eslintrc.json:::ES5;

classDef ES6 color:#3fb950
classDef ES5 color:#da3633

Loading

• We use the following plugins:
  • eslint-config-prettier - Turns off all rules that are unnecessary or might conflict with Prettier. I don't believe there is any change needed here.
  • eslint-config-es - We use several rules from this, but the key plugin that we need to remove is plugin:es/restrict-to-es5 that is appled in Source/.eslintrc.json

Exceptions

As of 1.93, we use disable eslint in our code for the following rules:

• no-cond-assign
• no-unused-vars
• global-require
• no-alert
• prefer-const
• new-cap
• no-empty
• no-self-assign
• no-undef
• no-math-sign
• no-math-sinh
• no-math-cosh
• no-math-cbrt
• no-math-log2
• no-lonely-if
• guard-for-in
• getter-return
• no-use-before-define
• no-implicit-globals
• no-global-assign
• no-else-return
• no-loop-func

Note: It was easy enough to find these because in most cases, we specify the rule being broken. However, in other cases, we just use eslint-disable-line. We should enforce that all rules being broken be explicitly mentioned.

Current Rules

At the moment, the primary source of ES6 restrictions come from the eslint-plugin-es plugin. Specifically, from the use of the plugin:es/restrict-to-es5 configuration. From the source code of the module, we can see that this configuration is composed of the following plugins:

• plugin:es/no-new-in-es2020
• plugin:es/no-new-in-es2019
• plugin:es/no-new-in-es2018
• plugin:es/no-new-in-es2017
• plugin:es/no-new-in-es2016
• plugin:es/no-new-in-es2015

Since each of these is composed of many rules, it may be cumbersome to create a PR for each one. Therefore, I propose that we incrementally work through each of these plugins - with a PR for removing the restrictions for each one, eventually ending up with support for ECMAScript 2020, which is what we should set our ecmaVersion to.

Additionally, going through the ESLint documentation for environments, I noticed that the jasmine environment only adds global variables for version 1.3 and 2.0. There is a more up to date plugin called eslint-plugin-jasmine. I don't want to update for the sake of updating, but I think it may be worth investigating once the core upgrades are done.

Another step may be to use eslint-plugin-jsdoc to ensure consistency in our documentation.

Proposed Rules

I went through all the rules available in eslint-plugin-es and found some rules that I think we may want to impose:

• ES2015
  • es/no-classes
  • es/no-computed-properties
  • es/no-generators
  • es/no-property-shorthands
  • es/no-reflect
  • es/no-rest-parameters
  • es/no-unicode-codepoint-escapes
• ES2017
  • es/no-async-functions
  • es/no-atomics
  • es/no-trailing-function-commas
• ES2018
  • es/no-async-iteration
  • es/no-malformed-template-literals
  • es/no-rest-spread-properties
• ES2019
  • es/no-json-superset

Ideally, anything we exclude we exclude with good reason. I selected these on first impression and am hoping I can get more feedback on what we should allow or not.

Upgrade coding guide

• We should confirm our theories on what ES6 features will potentially impact performance and upgade the coding guide accordingly.

[ggetz]

Inside CesiumJS, there are 6 configurations:

While I think its definitely possible to cut down on the number here, we will most likely still need a few variations based on the target environment. As part of this plan, can we recommend a new simplified "dependency tree"?

[sanjeetsuhag]

@ggetz Could you review the updated comment above? I've added my ideas for how we should proceed with these changes and compiled a list of rules we may want to impose.

[ggetz]

Before we get too deep into what rules we plan on adding, I would suggest we streamline our existing configs first. That would include turning off plugin:es/restrict-to-es5. I believe if we just include the es plugin, it will default to restricting to our target ecmaVersion, which should currently be 2015/es6. Then we can remove the rules we've explitely turned off for the es6 features we've allowed so far. We should also audit the Apps/Source/Specs configurations, and make sure they are using the same ecmaVersion and rules.

There is a more up to date plugin called eslint-plugin-jasmine.
Another step may be to use eslint-plugin-jsdoc to ensure consistency in our documentation.

More linting types certainly isn't a bad idea, but is tangential to this effort. Let's come back to those after first streamlining our configuration. Perhaps open a separate issue.

[ggetz]

I believe any actionable items here have been completed moved to other issues.