Skip to content

Conversation

@ggetz
Copy link
Contributor

@ggetz ggetz commented Sep 4, 2025

Description

Cleans up some rough edges that I overlooked in #12854.

  • Fixes build paths so that output is put in the right place for both root cesium development as well as sandcastle development.
  • Fixes the gallery file watcher and rebuild which is part of npm start.
  • Does a bit of cleanup to build.js while I was in there: I noticed that it was running some async functions as a side effect. That means they were running even when importing a totally unrelated function.

Issue number and link

Fixes #12867

Testing plan

  1. From a clean repository state, run npm install and npm run build. The gallery files should be built to Apps/Sandcastle2, not a parent directory as in the original bug report.
  2. Run npm start. Update a gallery file in packages/sandcastle/gallery and be sure the updates are rebuilt and reflected in http://localhost:8080/Apps/Sandcastle2/index.html.
  3. Change your working directory to packages/sandcastle. Ensure that running npm run dev build the gallery files to the packages/sandcastle/public directory, and that the app loads examples as expected.

Author checklist

  • I have submitted a Contributor License Agreement
  • I have added my name to CONTRIBUTORS.md
  • I have updated CHANGES.md with a short summary of my change
  • I have added or updated unit tests to ensure consistent code coverage
  • I have updated the inline documentation, and included code examples where relevant
  • I have performed a self-review of my code

@ggetz ggetz marked this pull request as ready for review September 4, 2025 21:30
@github-actions
Copy link

github-actions bot commented Sep 4, 2025

Thank you for the pull request, @ggetz!

✅ We can confirm we have a CLA on file for you.

@ggetz ggetz mentioned this pull request Sep 4, 2025
6 tasks
@ggetz ggetz requested a review from jjspace September 4, 2025 21:32
@ggetz ggetz self-assigned this Sep 5, 2025
@jjhembd
Copy link
Contributor

jjhembd commented Sep 12, 2025

On my WSL Ubuntu build, this PR fixes the issue from #12867:

  • No spurious Apps directory created outside the cesium folder
  • Working "Sandcastle2" link on the localhost page

Copy link
Contributor

@jjspace jjspace left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @ggetz sorry this took a bit to get to.

I think the code changes look good. Maybe small reworks as we do other build changes but these fix the current issues.
With the confirmation from @jjhembd that this works on Windows I'm happy to merge.

@jjspace
Copy link
Contributor

jjspace commented Sep 12, 2025

Actually @ggetz I went to get a picture of the slow gallery list build time I mentioned in person and realized there's an issue with the local server rebuild. If you spam save on a file (or even just hit it twice accidentally before it finishes) multiple builds get kicked off at the same time and they conflict. Sometimes it seems to catch it, sometimes it crashes fully.

2025-09-12_17-17
  1. I think we should revisit and speed this up. I suspect it's mostly pagefind so maybe we can add an option to skip that if a yaml file wasn't changed?
  2. I think we may want to add a better catch or prevent multiple runs at once or something.

Co-authored-by: jjspace <8007967+jjspace@users.noreply.github.com>
@ggetz
Copy link
Contributor Author

ggetz commented Sep 17, 2025

there's an issue with the local server rebuild. If you spam save on a file (or even just hit it twice accidentally before it finishes) multiple builds get kicked off at the same time and they conflict. Sometimes it seems to catch it, sometimes it crashes fully.

I'll take a look and see if we can make a small change here to prevent the crash.

I do agree that outside of this PR, we can take a pass and see if there's something we can do to speed up the overall build step.

@ggetz
Copy link
Contributor Author

ggetz commented Sep 17, 2025

@jjspace Updated!

  • Added a small timeout to prevent multiple rebuilds when saving in quick succession
  • Cleaned up error handling so that if the build function does throw, such as when missing a required property in sandcastle.yml, it is logged but does not stop the watcher or server.

Copy link
Contributor

@jjspace jjspace left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, may want further adjustments in the future but this seems fine for now

@jjspace jjspace added this pull request to the merge queue Sep 19, 2025
Merged via the queue into main with commit 7bd358c Sep 19, 2025
9 checks passed
@jjspace jjspace deleted the gallery-build-path-fix branch September 19, 2025 14:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Build process tries to create directory outside of Cesium directory

4 participants