Skip to content

feat(sveltekit): Streamline build logs - #17306

Merged
mydea merged 5 commits into
developfrom
fn/sveltekit-build-logs
Aug 7, 2025
Merged

feat(sveltekit): Streamline build logs#17306
mydea merged 5 commits into
developfrom
fn/sveltekit-build-logs

Conversation

@mydea

@mydeamydea commented Aug 4, 2025

Copy link
Copy Markdown
Member

This streamlines the build logs for sveltekit. For this, I aligned how we handle source maps setting with other SDKs, which is a bit different then how sveltekit did it.

  1. Only show log messages when debug: true
  2. Show short warning when sourceMaps: false and debug: false, more details when debug: true is set
  3. Re-organize how filesToDeleteAfterUpload is set, based on other SDKs (?? I hope that makes sense....)

@mydea
mydea requested a review from Lms24August 4, 2025 12:38
@mydeamydea self-assigned this Aug 4, 2025
cursor[bot]

This comment was marked as outdated.

@mydea

mydea commented Aug 4, 2025

Copy link
Copy Markdown
MemberAuthor

wait this does not work, let me draft it again...

@mydea
mydea marked this pull request as draft August 4, 2025 13:59
@mydea
mydea marked this pull request as ready for review August 5, 2025 08:35
cursor[bot]

This comment was marked as outdated.


const { promise: filesToDeleteAfterUpload, resolve: resolveFilesToDeleteAfterUpload } =
createFilesToDeleteAfterUploadPromise();
let _resolveFilesToDeleteAfterUpload:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I wonder if we should create a utility that does this, but then does a node version check to run Promise.withResolvers

Comment threadpackages/sveltekit/src/vite/sourceMaps.ts Outdated
cursor[bot]

This comment was marked as outdated.

@mydea
mydeaforce-pushed the fn/sveltekit-build-logs branch from 3d12225 to 66636caCompareAugust 6, 2025 11:22

@Lms24Lms24 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for cleaning this up and sorry for the late review! LGTM!

Sign up for freeto 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.

3 participants

@mydea@Lms24@AbhiPrasad