-
Notifications
You must be signed in to change notification settings - Fork 3.6k
Publish smaller packages #10636
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
Another benefit this may have is that it would allow us to upgrade our testing tools. We could take advantage of something like Jest, which could run tests in parallel. For stuff that needs to be run in a browser, we could use Playwright or Puppeteer and run tests in all browsers - potentially allowing us to do screenshot testing for things like Sandcastle. |
For the first iteration of this, we'll focus on creating a separate non-widget, engine-only package and a widgets package dependent on the former. This will allow us to set up the repo to support publishing multiple modules while focusing on one set. Separating out Packages (exact naming TBD) graph TD;
cesium/widgets-->cesium/engine
cesium-->cesium/engine
cesium-->cesium/widgets
cesium/engine-->deps[other npm dependencies]
cesium/widgets-->knockout
cesium/widgets-->nosleep
New packages should be added as an npm
Some things we'll need to consider for each package:
|
@ggetz This is exciting to see! Is my understanding correct on these details?
|
Yes. I think it would be cleaner to move source files directly into this directory structure rather than having scripts copy or reference code from
Yes, and this would be split along architectural boundaries such that units of code which require additional dependencies are separated into packages. To do so, we would increment a major version to denote the breaking change of modules moving to a separate package, and potentially |
@ggetz the initial plan looks great 💯 It may be a bit too early for this but maybe it would make sense to publish the new packages under the Regarding the testing/linting process, I think it would be better to have tests alongside the code of each package so that it will be easier to find it. Thanks for the effort 🙏 |
I agree that switching to a I'm definitely on-board with the overall intent of the plan, but I wonder if we have actually done any small prototypes (either hacked up Cesium or a small POC complete separate from Cesium) of how this might work. I feel like there could be a lot of potential showstoppers (like JSDoc, TS generation) hiding here that will require some significant work. A small prototype will help expose those problems up front.
Do we even need this? Cesium's VR support is ancient and I believe that's the only place this is used. I would definitely encourage us to take a second look at dependencies like this and figure out if the easier solution is to just delete code or remove a feature. How many people are actually using the Cesium VR support that exists today? (and is nosleep even needed for it).
This alone definitely feels a bit too coarse for me. @kring might have some thoughts here but to me it makes sense that the general rule should be "new dependency, new module" so those geojson/kml/google-earth etc.. modules make a ton of sense for keeping things lean and mean. @ggetz overall this is a great write-up and looking forward to seeing some proof-of-concepts. |
One other thing that comes to mind is public vs private APIs. Right now we have some bad practices where there is a lot of private API usage of the Cesium library. Would moving to modules have the rule that in order for one Cesium module to use the API of another, it has to be public? That seems the only way to dogfood things properly and make a good product. For example, I think even some low level utilities like |
I don't have too much to add. Since you're planning to stick with one repo (makes sense to me), there's not a lot of cost to extra libraries, so generally going "too fine" is better than "too coarse" IMO. For example, this long-ish list of libraries in cesium-native: (and that's even though CesiumJS does a lot that cesium-native doesn't, like all the GeoJSON / KML / Google Earth stuff Matt mentioned doesn't exist at all in native) But it also seems reasonable to me to start by breaking out some coarse pieces - even as coarse as engine and widgets, potentially - with the plan to break each of them up further into smaller pieces in the future. I don't think it will be too hard to maintain (temporary) backward compatibility during this evolution. |
To publish with the
@sanjeetsuhag Is currently working on this and we'll update here with results.
Yes, this is only used in the existing VR support. We'll consider removing if VR is not getting much usage. |
I am currently prototyping on the At the moment, I have just broken up the Here's a TODO list of things to test before we can begin to shape this into a PR:
|
There hasn't been sooo much feedback on the forum thread. But there's one point that I already mentioned in the forum, and that I wanted to bring up here, in view of the questions about "public vs private APIs" and the "long-ish list of libraries in cesium-native": Will there be an attempt to align the package structure between CesiumJS and cesium-native?It is phrased as a question, but from a very high-level perspective, there are good reasons to do that:
One should not be tooo strict here. And of course, this should not mean to blindly translaterate the API or class structures: The languages and environments are just too different, and in many cases, there will be judgement calls about the exact contents, scope and structure of a library. But for example, when it comes to questions like "Does 'BoundingRegion' belong into 'Geometry' or 'Geospatial'?", one could have a look at cesium-native and see whether it's possible to align both environments on the conceptual level. |
Request for a more minimal lighter-weight distribution requested in #11323 |
I cannot speak about a specific strategy or timeline, but am strongly in favor of trying to proceed with that. The way how gltf-pipeline is integrated always bugged me (some reasons recently summarized in #9614 (comment) ). So it's not really a "strategy", but ... maybe self evident... when I say that one step could be to try and figure out what exactly gltf-pipeline is using from Cesium that prevents Cesium using gltf-pipeline as a clean dependency (as there seems to be some circular dependency somewhere, and to break this, some "building block" has to be carved out). I'll still have to read the PR that you linked to more closely. Maybe related: There is a branch that extracts... some form of "core" module, at #8359 (comment) , but similarly, I have not (yet) looked at the details. |
@javagl , concerning the features used by gltf-pipeline from Cesium, here's the complete list:
All of which are from the Core part of @cesium/engine. So strictly speaking, detaching a @cesium/core component would be enough to break this circular dependency. graph TD;
gb[["`**bin**: gltf-pipeline`"]]
gl[@gltf-pipeline/lib]
gp[["`**lib**: gltf-pipeline`"]]
gc[@gltf-pipeline/core]
c[[cesium]]
ce[@cesium/engine]
cc[@cesium/core]
cw[@cesium/widgets]
c-->ce;
c-->cw;
ce-->gc;
ce-->cc;
cw-->ce;
cw-->cc;
gb-->gl;
gp-->gl;
gp-->gc;
gc-->cc;
My main concern is rather if we should split deeper this core component (eg, with a @cesium/math) or should it be done in a next step ? graph TD;
c[cesium]
ce[@cesium/engine]
cc[@cesium/core]
cm[@cesium/math]
cw[@cesium/widgets]
c-->ce;
c-->cw;
ce-->cc;
cw-->cc;
cw-->ce;
ce-->cm;
cc-->cm;
|
At one point, CesiumJS was broken into As mentioned in the forum thread: I think that eventually, there should be a structural similarity between the packages of CesiumJS and For example: When there's a class like So in general, I assume that the only practical way of creating these packages is to do this incrementally. Packages like
The "clean" packages should then contain only the things of which we are absolutely and 100% certain that they will never be removed (and preferably not even changed). The mantra from one of my favorite talks about API design that is applicable here is:
You can always add things later (in a non-breaking way), but you can never remove or change something without breaking clients. And for the More specifically about gltf-pipeline: Looking at the list, I think that such a
What's left is what clearly belongs into And more generically again: One of the main challenges for breaking CesiumJS into smaller packages is that the dependencies are not cleanly modelled. There are functions that have 800 lines of code and do 10 different things. So there could be a single function that requires access to maybe 8 different "packages", if these could be clearly defined to begin with. Even worse: There are single lines that So one question is: How did you determine the "List of things from CesiumJS that are used in gltf-pipeline"? (I think I also collected this list once, and I know that in this case, it's pretty easy to do this manually. In doubt, just remove the CesiumJS dependency and see where the compiler complains 😁 But I'm wondering if there is any form of tool that could help to automate that...) |
That's a long answer @javagl 😆 !
Simply |
Ok so let me try to summarize your thoughts @javagl :
Am I getting it right?
This is not so easy in my understanding. Taking the math example: as you move math functions from @cesium/engine to @cesium/math, you're only adding features to @cesium/math, so no breaking change here. But at one point, you'll have to deprecate those functions from @cesium/engine, so whatever you do, you'll always introduce breaking changes somewhere... |
That's what I'd consider a reasonable approach, but ... let's give other a chance to chime in here.
Yes, there will be breaking changes, on many levels (and the exact deprecation process has to be sorted out). But one important goal (that I could only describe somewhat abstractly) is about stabilities in the resulting dependency hierarchy. There are areas in the code where new features are added frequently (think about rendering glTF files and adding new extensions as they are ratified). There are areas in the code that will basically never change (e.g. math stuff like |
I think the brings up a big topic that will need to be considered here: What feature set can these lower-level packages depend on? Are they required to run in both the browser and in NodeJS? Should they target a lowest common denominator for browsers, or can we assume they will be bundled and transpiled? |
I don't have the full picture of what exactly is working where. Only roughly: Do something with But I think that for many packages (like Maybe this also helps to raise awareness. Maybe it's possible to introduce some abstractions (like WebIO and NodeIO in glTF-Transform). And maybe it's possible to keep the main part of the package compatible with both, and add some tiny add-on for the Platform-specific part - like |
@jfayot @javagl Specifically in regards to gltf-pipeline, we have begun some initial conversations in CesiumGS/gltf-pipeline#670, the results of which will determine the plan going forward for how we deal with the cyclical dependencies between that and CesiumJS. |
CesiumJS is an extensive library with a lot of functionality that, depending on the app or use case, goes unused. Installing the
cesium
npm package also syncs third party libraries which may go unused. All features are tied to the monthly Cesium versioning, whether or not anything has changed.Many users should continue to use the "traditional" CesiumJS releases, but there are also many use cases that would benefit from ingesting smaller, more specific packages.
See this community forum thread for some seeds of this discussion.
cesium
npm package and release would remain as-isnode_modules
directoryA few use cases:
CesiumViewer
and other Widgets work well for simple use cases that want to get something really basic up quickly. But a more sophisticated app does not need them, and they can actively be a hindrance as they bring in many static assets.CesiumWidget
does not rely on knockout.The text was updated successfully, but these errors were encountered: