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

Move --sourcemap to scripts instead of Invoke-Build #4707

Merged
merged 1 commit into from
Aug 25, 2023

Conversation

JustinGrote
Copy link
Collaborator

@JustinGrote JustinGrote commented Aug 20, 2023

PR Summary

ESBuild does not currently generate sourcemaps for debugging. This enables that generation and I verified that vsce does not include the sourcemaps when packaging.

PR Checklist

Note: Tick the boxes below that apply to this pull request by putting an x between the square brackets.
Please mark anything not applicable to this PR NA.

  • PR has a meaningful title
  • Summarized changes
  • PR has tests
  • This PR is ready to merge and is not work in progress
    • If the PR is work in progress, please add the prefix WIP: to the beginning of the title and remove the prefix when the PR is ready

@JustinGrote JustinGrote requested a review from a team August 20, 2023 19:42
@JustinGrote JustinGrote self-assigned this Aug 20, 2023
@andyleejordan
Copy link
Member

Isn't evanw/esbuild#944 an issue?

# TODO: When supported we should use `esbuild` for the tests too. Although
# we now use `esbuild` to transpile, bundle, and minify the extension, we
# still use `tsc` to transpile everything in `src` and `test` because the VS
# Code test runner expects individual files (and globs them at runtime).
# Unfortunately `esbuild` doesn't support emitting 1:1 files (yet).
# https://github.com/evanw/esbuild/issues/944
switch ($Configuration) {
"Debug" { Invoke-BuildExec { & npm run build -- --sourcemap } }
"Release" { Invoke-BuildExec { & npm run build -- --minify } }
}
}

@andyleejordan
Copy link
Member

The build script adds --sourcemap on debug and --minify on release.

@JustinGrote
Copy link
Collaborator Author

I'm using build watch and not the build script, that's probably why, it happens with "run extension" but not run tests because the file that it uses to run is different for both. Let me think about this tomorrow, I have an idea how to do it better.

So that they're generated even when using `build-watch`. Double-checked
that they're not included in the package (even though they're now
generated for release configuration too).
@andyleejordan andyleejordan force-pushed the feature/esbuild-sourcemaps branch from 32a097e to 368bbb9 Compare August 25, 2023 16:50
@andyleejordan
Copy link
Member

@JustinGrote I gotchu.

@andyleejordan andyleejordan changed the title Add sourcemap generation to esbuild Move --sourcemap to scripts instead of Invoke-Build Aug 25, 2023
@andyleejordan andyleejordan added Issue-Enhancement A feature request (enhancement). Area-Build & Release labels Aug 25, 2023
@andyleejordan andyleejordan added this pull request to the merge queue Aug 25, 2023
Merged via the queue into main with commit 4994959 Aug 25, 2023
@andyleejordan andyleejordan deleted the feature/esbuild-sourcemaps branch August 25, 2023 17:12
@JustinGrote
Copy link
Collaborator Author

@andyleejordan thanks! I was going to just put it in the watch npm script but as long as it doesn't hurt anything in main (it shouldn't since the tsc source maps shouldn't conflict) I'm good with it there, they take an insignificant amount of additional time to generate.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Build & Release Issue-Enhancement A feature request (enhancement).
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants