Skip to content
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

fix(sveltekit): Properly await sourcemaps flattening #10602

Merged
merged 1 commit into from
Feb 12, 2024

Conversation

Lms24
Copy link
Member

@Lms24 Lms24 commented Feb 12, 2024

As pointed out in #10589, we didn't properly await sourcemaps flattening via sorcery before proceeding to upload them. The reason is that the async callbacks in forEach weren't awaited. A for loop is the better approach here. Wondering if we should lint against async forEach callbacks.
This behaviour could have caused various inconsistencies. My suspicion is that the timing worked well enough in most cases but we definitely want to properly await this step.

Thanks to @MSDev201 for bringing this up!

Unfortunately this likely won't fix #10589 as a whole :(

Copy link
Contributor

size-limit report 📦

Path Size
@sentry/browser (incl. Tracing, Replay, Feedback) - Webpack (gzipped) 78.1 KB (-0.08% 🔽)
@sentry/browser (incl. Tracing, Replay) - Webpack (gzipped) 69.31 KB (-0.09% 🔽)
@sentry/browser (incl. Tracing, Replay with Canvas) - Webpack (gzipped) 73.25 KB (-0.09% 🔽)
@sentry/browser (incl. Tracing, Replay) - Webpack with treeshaking flags (gzipped) 62.95 KB (-0.1% 🔽)
@sentry/browser (incl. Tracing) - Webpack (gzipped) 33.27 KB (-0.25% 🔽)
@sentry/browser (incl. browserTracingIntegration) - Webpack (gzipped) 33.13 KB (-0.25% 🔽)
@sentry/browser (incl. Feedback) - Webpack (gzipped) 31.15 KB (-0.15% 🔽)
@sentry/browser (incl. sendFeedback) - Webpack (gzipped) 31.15 KB (-0.19% 🔽)
@sentry/browser - Webpack (gzipped) 22.4 KB (-0.35% 🔽)
@sentry/browser (incl. Tracing, Replay, Feedback) - ES6 CDN Bundle (gzipped) 76.08 KB (-0.15% 🔽)
@sentry/browser (incl. Tracing, Replay) - ES6 CDN Bundle (gzipped) 67.62 KB (-0.14% 🔽)
@sentry/browser (incl. Tracing) - ES6 CDN Bundle (gzipped) 33.37 KB (-0.32% 🔽)
@sentry/browser - ES6 CDN Bundle (gzipped) 24.46 KB (-0.41% 🔽)
@sentry/browser (incl. Tracing, Replay) - ES6 CDN Bundle (minified & uncompressed) 213.23 KB (-0.21% 🔽)
@sentry/browser (incl. Tracing) - ES6 CDN Bundle (minified & uncompressed) 101.03 KB (-0.44% 🔽)
@sentry/browser - ES6 CDN Bundle (minified & uncompressed) 73.42 KB (-0.6% 🔽)
@sentry/browser (incl. Tracing) - ES5 CDN Bundle (gzipped) 36.54 KB (-0.28% 🔽)
@sentry/react (incl. Tracing, Replay) - Webpack (gzipped) 69.71 KB (-0.09% 🔽)
@sentry/react - Webpack (gzipped) 22.45 KB (-0.3% 🔽)
@sentry/nextjs Client (incl. Tracing, Replay) - Webpack (gzipped) 87.45 KB (-0.1% 🔽)
@sentry/nextjs Client - Webpack (gzipped) 50.35 KB (-0.14% 🔽)
@sentry-internal/feedback - Webpack (gzipped) 17.12 KB (-0.48% 🔽)

@Lms24 Lms24 requested review from mydea and lforst February 12, 2024 10:16
@Lms24 Lms24 merged commit d2ef1cb into develop Feb 12, 2024
53 checks passed
@Lms24 Lms24 deleted the lms/fix-sveltekit-flattening branch February 12, 2024 10:52
Lms24 added a commit that referenced this pull request Feb 12, 2024
We didn't properly await sourcemaps flattening via sorcery before proceeding to
upload them. The reason is that the async callbacks in `forEach` weren't
awaited. A `for` loop is the better approach here. Wondering if we
should lint against async `forEach` callbacks.
This behaviour could have caused various inconsistencies. My suspicion
is that the timing worked _well enough_ in most cases but we definitely
want to properly await this step.

Thanks to @MSDev201 for bringing this up!

Unfortunately this likely won't fix #10589 as a whole :(
Lms24 added a commit that referenced this pull request Feb 12, 2024
We didn't properly await sourcemaps flattening via sorcery before proceeding to
upload them. The reason is that the async callbacks in `forEach` weren't
awaited. A `for` loop is the better approach here. Wondering if we
should lint against async `forEach` callbacks.
This behaviour could have caused various inconsistencies. My suspicion
is that the timing worked _well enough_ in most cases but we definitely
want to properly await this step.

Thanks to @MSDev201 for bringing this up!

Unfortunately this likely won't fix #10589 as a whole :(
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.

Sveltekit - Ineffective mark-compacts near heap limit Allocation failed - JavaScript heap out of memory
2 participants