-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
Streamline build processes with esbuild #10399
Merged
Merged
Changes from all commits
Commits
Show all changes
43 commits
Select commit
Hold shift + click to select a range
ff559c7
build workers, third party, and combine with eslint
ggetz 4bb8024
combine output
ggetz 355dd42
Updated build process
ggetz 54ef709
Spec and worker fixes
ggetz eb60fbf
cleanup build tasks
ggetz f6cf110
clean up gulp tasks and naming
ggetz bddf66e
Remove requireJS from LICENSE and ThirdParty.json
ggetz e71c886
Update build guide
ggetz eb87e96
Tweak travis file
ggetz 44b9610
karma specs and coverage
ggetz 0871e6a
sourcemap by default for dev only
ggetz 27bbdbf
Tweaks for CI
ggetz ec317da
Sandcastle
ggetz 16a9c80
Fix up build output names and apps
ggetz 7ac3fd0
coverage
ggetz d867651
Tweak specs
ggetz 290bed0
Tweak build guide
ggetz 3938feb
Tweak tests
ggetz 4ecefb5
Merge branch 'main' into build
ggetz 867ab1a
Cleanup documentation
ggetz 6404f52
Test cleanup
ggetz 1f99b62
Cleanup
ggetz 018aca6
Tweak watched files
ggetz 3293a5b
Cleanup coverage
ggetz 4cf8ba1
Legal comments
ggetz 318c276
Cleanup rollup dependencies
ggetz 6a31cf1
Clarify documentation
ggetz 8d7faae
Merge branch 'main' into build
ggetz 312cb98
Fix build error and warning
ggetz 9905c12
Merge branch 'main' into build
ggetz 1fbc968
2020
ggetz 213cdfc
Rollback script naming changes
ggetz 6ae6cda
prettier
ggetz 8bc8355
Remove worker changes
ggetz 3a8e5c8
Remove extraneous "the"
ggetz 0a19ae3
Merge branch 'main' into build
ggetz 095033d
Update CHANGES.md
ggetz 17babe9
Tweak workers, remove pervasive spread operations
ggetz ad0dc45
prettier
ggetz 24c4590
Merge branch 'main' into build
ggetz d6b9a96
Fix typo
ggetz c575a3f
Feedback update
ggetz 7fd59fd
Tweak node target
ggetz File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
window.CESIUM_BASE_URL = "."; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,6 +5,10 @@ | |
##### Breaking Changes :mega: | ||
|
||
- Tilesets and entities now use `ModelExperimental` by default. This can still be changed by setting `ExperimentalFeatures.enableModelExperimental = false`. [#10530](https://github.com/CesiumGS/cesium/pull/10530) | ||
- Built `Cesium.js` is no longer AMD format. This may or may not be a breaking change depending on how you use Cesium in your app. See our [blog post](TODO) for the full details. [#10399](https://github.com/CesiumGS/cesium/pull/10399) | ||
- If you were ingesting individual ESM-style modules from the combined file `Build/Cesium/Cesium.js` or `Build/CesiumUnminified/Cesium.js`, instead use `Build/Cesium/index.js` or `Build/CesiumUnminified/index.js` respectively. | ||
- `CESIUM_BASE_URL` should be set to either `Build/Cesium` or `Build/CesiumUnminfied`. | ||
- Built `Cesium.js` has gone from `12.5MB` to `8.4MB` unminified and from `4.3MB` to `3.6MB` minified. `Cesium.js.map` has gone from `22MB` to `17.2MB`. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should probably be absolutely top of the list with 📣, it's not a breaking change and it's a huge deal. |
||
|
||
##### Additions :tada: | ||
|
||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doesn't Cesium now 100% require a bundler to use modules? (And a bundler that allows for node resolution rules at that). Or are we still embedding third party code?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We're embedding the third-party code in these builds. In the blog post I mention that production apps that use their own bundler should be using modules directly
Source
, which does not have third party code bundled. Which is what we've previously recommended as well if I understand correctly.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have previously recommended it, but since we embedded ThirdParty code in source, you didn't need a bundler to use them. Now you do.
Another problem I just noticed is that
package.json
lists everything as a dev dependency, which may not be correct because that means if Inpm install Cesium
and then try to useSource
it's going to pull in modules that don't exist because they are not adependency
orpeerDependency
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Gotcha, I see the distinction. For the point about the bundler, I will update both
CHANGES.md
and the blog post with the requirement.For the latter, I will
package.json
such that modules inSource/ThirdParty
are now marked as direct dependencies. Sound correct?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. It's unfortunate because people who want to use the bundled Cesium.js still need to sync those dependencies but there's no other way that I know of to do it and have all usage-types work.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey all, speaking of
package.json
, would this be a good time to address #9212 as well? It's really important to get the exports right, I had a workaround but had to get rid of it to be able to use another important dependency at all...ETA: to clarify, any resource that you intend for people to be able to bundle or extract separately (CSS, images, maybe data files like terrain height / IAU JSON) should have its own
exports
entry, and excluding those entries makes consumption of the resources effectively impossible.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @thw0rted, thanks for pointing this out! We have some additional thoughts on related improvements beyond just this PR, including considering #9212. We'll add our thoughts or any updates to that issue.