Skip to content

chore: remove sourcemaps for bundled files#4049

Merged
domoritz merged 1 commit intomainfrom
dom/maps
Mar 30, 2025
Merged

chore: remove sourcemaps for bundled files#4049
domoritz merged 1 commit intomainfrom
dom/maps

Conversation

@domoritz
Copy link
Copy Markdown
Member

@domoritz domoritz commented Mar 29, 2025

They don't really help since we don't ship the sources in the package anyway. If you want sourcemaps, use ESM.

This change should make our packages smaller. See https://packagephobia.com/result?p=vega

They don't really help since we don't ship the sources in the package anyway. If you want sourcemaps, use ESM.
@domoritz domoritz requested a review from a team as a code owner March 29, 2025 13:39
@domoritz domoritz enabled auto-merge (squash) March 29, 2025 16:27
Copy link
Copy Markdown
Member

@hydrosquall hydrosquall left a comment

Choose a reason for hiding this comment

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

I think a second impact is this will speed up the build times too! I don't have metrics before/after metrics on what this looked like before (perhaps this is something we could start tracking in github actions).

From past work, I've seen sourcemaps generation may add noticeable overhead to compile time, and that we might want to save those for dev rather than production mode.

Disk savings

image

It looks like we'll save close to 6 MB by not shipping sourcemaps per runpkg - nice!

image

Unfortunately the install footprint still seems like it's over 20 MB, but this might be something we get to address by addressing the maps on the various sub dependencies of vega

image

Meta

Node.js test is failing on vega-interpreter, but I think it's unrelated to this change, so we can investigate it separately.

@domoritz domoritz merged commit 21476c1 into main Mar 30, 2025
7 of 11 checks passed
@domoritz domoritz deleted the dom/maps branch March 30, 2025 16:55
@domoritz
Copy link
Copy Markdown
Member Author

Node.js test is failing on vega-interpreter, but I think it's unrelated to this change, so we can investigate it separately.

There is a flaky test. I just kept rerunning and then tests pass. Yes, would be lovely to investigate.

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.

2 participants