Node 16 will be LTS on `2021-10-26` so we need to ensure it's passing and
works.
We might want to consider testing against multiple Node versions down the
line, but using the latest LTS has worked well so far (though apparnetly
we skipped 14 completely here)
While we updated Cesium to work on 12, we never actually made CI use it.
Traditionally we always use the active LTS for CI (which covers 12 until
October when it becomes 14).
It's been a long requested feature for us to have official TypeScript type
definitions. While the community has done a yeoman's job of manually
supporting various efforts, the most recent incarnation of which is
`@types/cesium`, the sheer scale and ever-evolving nature of Cesium's code
base makes manual maintenance a Sisyphean task.
Thankfully, our decision to maintain meticulous JSDoc API documentation
continues to pay dividends and is what makes automatically generating
TypeScript definitions possible. Using the excellent
https://github.com/englercj/tsd-jsdoc project we can now automatically
generate and even partially validate official definitions as part of the
build process. (Thanks to @bampakoa who contributed some early PRs to both
CesiumJS and tsd-jsdoc over a year ago and is how I learned about
tsd-jsdoc)
While tsd-jsdoc output is mostly where we need it to be, we do
post-processing on it as well. This lets us clean up the output and also
make sure these definitions work whether users include cesium via module,
i.e. `import { Cartesian3 } from 'cesium'`, or individual files, i.e.
`'import Cartesian3 from 'cesium/Source/Core/Cartesian3'`. There were also
some quirks of tsd-jsdoc output we fixed that may eventually turn into a PR
into that project from us. The post-processing is part typescript compiler
API, part string manipulation. It works and is straightforward but we might
want to go full TS api in the future if we decide we need to do more
complicated tasks. The output of tsd-jsdoc is currently a little noisy
because of some incorrect error reporting, but I'm talking with the
maintainer in https://github.com/englercj/tsd-jsdoc/issues/133 to get them
fixed. No need to hold up this PR for it though.
The definition is generated as a single `Cesium.d.ts` file in Source, so it
lives alongside Cesium.js. It is ignored by git but generated by a separate
`build-ts` task as part of CI and makeZipFile. This task also validates the
file by compiling it with TypeScript, so if a developer does anything too
egregious, the build will fail. Definitions are automatically included in
our npm packages and release zips and will be automatically used by IDEs
thanks to the `types` setting in package.json. This means that IDEs such as
VS Code will prefer these types over the existing `@types/cesium` version
by default.
I didn't want to slow the `build` step down, so I made this a separate
step, but in the future we could consider running it by default and we
could also unignore this file in Git so that PR reviewers can see the
impact, if any, our code changes have on the generated definitions. This
might be a good idea as an additional sanity check and should only actually
change when the public API itself changes. But the issue would be
remembering to run it before submitting the code (or we could use git hooks
I suppose?) I just don't want to slow down devs so I'm hesitant to do
anything like this out of the gate. We can definitely revisit in the
future.
A particular exciting thing about this approach is that it exposed a ton of
badness in our current JSDoc markup, which is now fixed. Previously, our
only concern was "does the doc look right" and we didn't pay attention to
whether the meta information generated by JSDoc correctly captured type
information (because up until it didn't matter). We leaned particular hard
on `@exports` which acted as a catch-all but has now been completely
removed from the codebase. All this means is that our documentation as a
whole has now improved greatly and will continue to be maintained at this
new higher level thanks to incorporating TS definition creation into our
pipeline!
One minor caveat here is that obviously we changed our JSDoc usage to both
make it correct and also accommodate TypeScript. The main drawback to these
fixes is that enums are now generated as globals in the doc, rather than
modules. This means they no longer have their own dedicated page and are
instead on the globals page, but I changed the code to ensure they are
still in the table of contents that we generate. I think this trade-off is
perfectly fine, but I wanted to mention it since it does change the doc
some. We can certainly look into whether we can generate enums on their own
page if we think that makes sense. (I actually like this approach a little
better personally).
Last major piece, the actual code. 99% of the changes in this PR only
affect the JSDoc. There are two exceptions:
A few of our enums also have private functions tacked onto them. I had to
move these functions to be outside the initializer but otherwise they are
unchanged. This ensures that a valid TS enum is generated from our code, since you can't have functions globbed onto enums in the TS world. If we were writing TS code by hand, we could use declaration merging with a namespace, but the toolchain we are using doesn't have a way to express that right now. There were two cases where these extra functions weren't private, `ComponentDataType` and `IndexDataType`. That means that as far as the TS definitions goes, the helper methods don't exist. I consder this an edge case and we can write up issues to investigate later. I'm actually not even sure if these functions are public on purposes, @lilleyse can you confirm?
We had a few places where we had method signatures with optional parameters
that came _before_ required parameters, which is silly. This is invalid
TypeScript (and not good API design no matter the language). In 99% of
cases this was `equalsEpsilon` style functions where the lhs/rhs were
optional but the epsilon was not. I remember the discussion around this
when we first did it because we were paranoid about defaulting to 0, but
it's an edge case and it's silly so I just changed the epsilon functions
to default to zero now, problem solved.
Here's a high level summary of the JS changes:
* Use proper `@enum` notation instead of `@exports` for enums.
* Use proper `@namespace` instead of `@exports` for static classes.
* Use proper `@function` instead of `@exports` for standalone functions.
* Fix `Promise` markup to actually include the type in all cases, i.e.
`Promise` => `Promise<void>` or `Promise<Cartesian3[]>`.
* Fix bad markup that referenced types that do not exist (or no longer
exist) at the spec level, `Image` => `HTMLImageElement`,
`Canvas` => `HTMLCanvasElement`, etc.. `TypedArray` in particular does not
exist and much be expressed as a lsit of all applicable types,
`Int8Array|Uint8Array|Int16Array|Uint16Array...`.
* Use dot notation instead of tilde in callbacks, to better support
TypeScript, i.e. `EasingFunction~Callback` becomes
`EasingFunction.Callback`. The only exception is when we had a callback
type that was i.e. `exportKml~ModelCallback` becomes
`exportKmlModelCallback` (a module global type rather than a type on
exportKml). This is because it's not possible to have exportKml be both a
function and a namespace in this current toolchain. Not a big deal either
way since these are meta-types used for defining callbacks but I wanted to
mention it.
* There were some edge cases where private types that were referenced in
the public API but don't exist in the JSDoc. These were trivial to fix by
either tweaking the JSDoc to avoid leaking the type or in some cases, just
as `PixelDataType`, simply exposing the private type as public. I also
found a few cases where things were accidentally public, I marked these as
private (these were extreme edge cases so I'm not concerned about breaking
changes). Appearances took an optional `RenderState` in their options, I
just changed the type to `Object` which we can clean up further later if
we need to.
* Lots of other little misc JSDoc issues that became obvious once we
started to generate definitions (duplicate parameters for example).
Thanks again to the community for helping generate ideas and discussions
around TS definitions over the last few years and a big thanks to @javagl
for helping behind the scenes on this specific effort by evaluating a few
different approaches and workaround before we settled on this one (I'm
working on a blog with all of the gory details for those interested).
Finally, while I'm thrilled with how this turned out (all ~41000 lines
and 1.9MB of it), I can guarantee we will uncover some issues with the
type definitions as more people use it. The good news is that improving it
is now just a matter of fixing the JSDoc, which will benefit the community
as a whole and not just TS users.
Fixes#2730Fixes#5717
1. Add prettier and configuration files, but do not format code yet.
2. Configure it for all html|css|js|md code.
3. Fix a bug it found in `CZML Custom Properties.html`
4. Add a pre-commit hook via husky and pretty-quick which will
automatically format changed files when they are staged on the client.
5. Run `prettier-check` during CI and fail the build of code is not
formatted properly.
6. Install eslint config for prettier. This does not enable prettier
checking in eslint (it's unusably slow) but instead just makes sure that
no eslint rules conflict with prettier formatting.
7. Update .editorconfig to match (since prettier respects it)
8. Tweak Sandcastle to handle prettier formated examples
See https://github.com/AnalyticalGraphicsInc/cesium/pull/8224 for details.
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.
1. Add options from the `test` task to the `coverage` task
2. Run `coverage` instead of `test` on CI (since covereage includes text)
3. Add link to coverage results in index.html and also as part of the
the GitHub deploy information.
While the improvements in #7514 worked, they were a little more convoluted
than they had to be and also caused some weird edge cases where loading
Cesium in eval'd Node.js code didn't work.
This change avoids using requirejs entirely when loading minified Cesium
in Node and adds a smokescreen test to travis that fails in master. This
also avoids the odd global workaround I had to put in place previously.
I did have to modify the UMD headers for NoSleep.js and purify.js because
their preference is to look for Node.js first and not use AMD if it's
found, this was the root cause of the failures we were seeing in eval'd
code. The smokescreen test I added will fail again if the problem were
reintroduced.
So overall, this make Cesium a little more node-friendly and just makes
things cleaner from our end.
We know that removing debug code in Cesium can provide a substantial
speed improvement, but previously the Node.js version would always use
separate modules. This change allows Node.js to load the combined/minified
version by default, for improved performance, and only loads the old
separate modules if NODE_ENV=development (which is the standard mechanism
for enabling extra debugging checks in most node packages.
1. When running the unit tests from the command line, do not output karma
server info or browser console output. Now the only output are the tests
themselves (and whether they pass/failed). Use `--verbose` to get the
full output from before.
2. Deploy output only prints a summary now. Use `--verbose` to get the
full output from before.
3. Add `--silent` to npm travis calls, this just removes the npm
boilerplate output which is useless.
4. Use `travis_wait` instead of logging every file in `makeZipFile`. It
has the same affect in that it lets the build take more than 10 minutes
without timing out.
5. `makeZipFile` now outputs very little. Use `--verbose` to get the
full output from before.
6. Get rid of travis log folding because it's no longer needed.
1. `buildSandcastle` is returning a stream that is passed to Promise.join.
That means the promise resolves before the stream is finished.
Instead it should be returning a promise created from the stream.
2. `npm pack` may be screwed with travis' console leading to random
failures, redirect it's output to /dev/null to see if it addresses our
random build failures
3. requirejs was occasionally being killed by travis for using too much
memory, reduce concurrency during the build process to prevent this from
happening.
1. Don't use PointerEvents for tests in Firefox, see #6539. This makes
all tests but one pass.
2. Run built files against FirefhoxHeadless. This means we run unminified
on Chrome and minified on Firefox. This helps keep the build time down
by not running everything twice on both browsers, but will still catch
Firefox-specific bugs that we can then run locally.
Since Chrome added an officially supported headless mode, there's no
reason to use Electron for our unit tests. This will be both more accurate
(uses actual Chrome) and also gets rid of the rather large Electron
dependency.
WebGL is disabled in headless, so we only use headless on CI (because we
also disable webGL tests there). When running locally, it just shows an
actual Chrome window. Chrome has an issue and plans to address this:
https://bugs.chromium.org/p/chromium/issues/detail?id=765284
1. Start building on travis with Node 8 since it will be LTS soon (it
should slightly speed up builds as well)
2. Add package-lock.json to .gitignore, this is a new npm-generated file
that we won't be submitting to GitHub (it changes almost every time you run
`npm install`).
3. Add package.json to .gitattributes to fix line endings caused by this
npm bug: https://github.com/npm/npm/issues/17161
The problem was that travis itself was killing our build process because
we were trying to run 8 requirejs processes at once, leading us to run out
of memory (only 3GB available on travis). This adds a command line to
specify concurrency and updates travis to do at most 2 (one for each core).
1. Caching makes eslint only take ~3 seconds plus any files that have
changed since the last time you ran it. Since it's unlikely devs are
touching every tile between runs, this makes eslint much incredibly faster
in the average case. Also added the genereated `.eslintcache` to git
ignore.
2. Switched to the pure cli version of eslint and remove `eslint-watch`,
which I'm pretty sure no one uses anyway. This simplified our usage and
means we lint all js and html files by default except for the globs
specifically listed in `.eslintignore` This also shaves 2-4 seconds off
startup time because we're not loading gulpfile.js anymore.
3. Fixed an issue in `index.release.html`, which was previously not linted.
This change allows travis to deploy the generate zip files and
npm packages in addition to the build output. Each package
gets a unique name based on branch name and build number.
Also updated deployment step not to delete these packages so that
they can be depended on until the branch is merged.
The travis command was missing `--` which was causing it to not fail
in the event of an error. This also means we let a couple of warnings
slip into master (which I have also fixed).
1. Running travis under node 4.3.x gives a decent improvement over 0.12.
2. Move `cloc` to the end of the build since it's the least important.
3. Take advantage of multiple cores for combine/minification
Number 3 reduced `makeZipFile` times on my machine from 5:40 to 2:30,
performance increases has transferred to travis as well. Because
travis reports 32 cores and trying to use them all results in
travis killing the build, I put in a hardcoded limit of 8 for now.
My approach is a little hacky (I exec a seprate gulp task passing a base64
encoded string on the command line), but I couldn't determine a better
way to handle it with gulp because there's no programmatic way to spawn
a task (I think this is being fixed in gulp 4, so we'll revisit when that
comes out).
1. Run non-webgl tests as part of travis using Electron. They run in both
module and release form.
2. Run cloc as part of the build process
3. jsHint no longer fails the gulp task when you run `npm run jsHint`,
since that behavior was really annoying. It still fails under travis or
when passed the `--failTaskOnError` option.
4. Improve log output by using travis folding commands. Each part of the
build is now collapsed into its own section.
5. Added `--suppressPassed` option to avoid showing passed tests in output.
Our travis log got too big and travis refused to show it (even with the
folding) unless you looked at raw test.
Gabby Getz
Matthew Amato
Kevin Ring
Omar Shehata
Gabby Getz
hanbollar
Jason Wohlgemuth
Patrick Cozzi