CesiumGS / cesium-webpack-example

The minimal recommended setup for an application using Cesium with Webpack.
Apache License 2.0
243 stars 160 forks source link

Update to latest version of everything and Cesium via Node/ES6 #13

Closed mramato closed 4 years ago

mramato commented 4 years ago

I know almost nothing about webpack, but this updates it to use a pre-release package of Cesium with ES6 support (which will become 1.63 so we can wait until that is out before we merge this).

It also switches the demo to use the latest version of all libraries and resolve packages from node the way most JS libraries work these days.

I noticed the fireworks no longer work in master or in this branch, I think a more practical example that loads a tileset from ion or something may be a better and simpler dmoe; but that can be in another PR.

I'm sure there are other things that can be simplified here as well, but my main concern was just updating everything. Open to suggestions (or someone else can just pick up this PR and do more if they want).

My main goal here was to make sure that Cesium 1.63 still worked with webpack and it does (and in a better/more modern way than it did previously).

puckey commented 4 years ago

The cesium dependency in package.json could be moved from devDependencies to dependencies

puckey commented 4 years ago

A good guide to take a look at for updating from Webpack 3 to 4 would be this one: https://webpack.js.org/migrate/4/

For example, you can remove the UglifyJS plugin, because it is added by default in production: https://webpack.js.org/migrate/4/#deprecatedremoved-plugins

mramato commented 4 years ago

Thanks for the links @puckey

I figured out why the fireworks were broken we were missing a CopyWebpackPlugin entry for ThirdParty. I'm still going to simplify the example to just load a 3D Ties tileset since that is a little more useful for our users.

mramato commented 4 years ago

The cesium dependency in package.json could be moved from devDependencies to dependencies

What's your reason for doing this? Since this repository isn't meant to be depended on, I don't think it matters (and Cesium is only a compile time dependency because everything needed gets copied and hosted out of dist correct? Just want to make sure I'm not missing anything.

puckey commented 4 years ago

The cesium dependency in package.json could be moved from devDependencies to dependencies

What's your reason for doing this? Since this repository isn't meant to be depended on, I don't think it matters (and Cesium is only a compile time dependency because everything needed gets copied and hosted out of dist correct? Just want to make sure I'm not missing anything.

Technically, there is no reason to do so, but it is a somewhat of a practice in webpack land to put bundle-ing / transpiling packages in devDependencies and packages that contain code to be included in the compiled bundle in dependencies. Here are a bunch of starter kits, for example: https://webpack.js.org/starter-kits/

One of the first things I would do as a user is to take a peek at the dependencies to see what is being included in the build.

There would also be something to be said for putting everything into dependencies, since they are installed no matter what the value is of NODE_ENV, unlike devDependencies which are ignored when installed with NODE_ENV set to production.

mramato commented 4 years ago

README.md will need updates. We should at least update the bundle size comparisons, and probably reorder the "Requiring" Cesium section so that ES6 imports are first.

Looking at updating the readme now. I don't think we should recommend anyone bring in individual modules, we make no guarantees that filenames won't change over the course of a release. I think that we should message ES6 as the only preferred way and they should use standard syntax like import { Viewer } from 'cesium'; instead of trying to import based on path.

mramato commented 4 years ago

bundle size comparisons,

Also, because we are always using ES6 imports from the primary cesium object, there's no reason for this comparison because a proper webpack setup will treeshake away the unused modules.

mramato commented 4 years ago

@ggetz all initial feedback has been fixed and I made a bunch of other improvements

  1. Add mode flag to each webpack config
  2. Fix third party workers and wasm modules
  3. Remove sourcePrefix since an empty string is now the default
  4. Remove uglify because it's no longer needed.
  5. Enable tree-shaking in release.
  6. Enable source-maps by default in development.
  7. Pare down README
  8. Switch to a more basic example application and link to the Getting Started.

Let me know if there's anything you think we really need to add back (though I'm not sure how much more time I can spend on this)

mramato commented 4 years ago

@ggetz bump. We're linking to this repository in the ES6 blog post that will go out on Thursday so I would like to get this merged by then (I will then do a small update to point to CesiumJS 1.63 officially when we release on Friday).

ggetz commented 4 years ago

These modifications look great! Thanks for the much-needed updates to this repo @mramato !