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

Use the right configuration option for inline Typescript sourcemaps #1135

Closed
wants to merge 1 commit into from

Conversation

daaain
Copy link

@daaain daaain commented Jul 29, 2019

Using the current recommended settings I had source map files generated separately alongside compiled source files. Changing the sourceMap option to inlineSourceMap made it work.

It sounds like it should work as it is, but what actually happens is that there's only the file path of the separate source map file emitted in the source files.

For reference: https://www.typescriptlang.org/docs/handbook/compiler-options.html

Option Type Default Description
--inlineSourceMap boolean false Emit a single file with source maps instead of having a separate file.
--inlineSources boolean false Emit the source alongside the sourcemaps within a single file; requires --inlineSourceMap or --sourceMap to be set.
--sourceMap boolean false Generates corresponding .map file.

@dashed dashed requested a review from kamilogorek July 29, 2019 16:13
@kamilogorek
Copy link
Contributor

It sounds like it should work as it is, but what actually happens is that there's only the file path of the separate source map file emitted in the source files.

Works just fine for me. Here's verified repo and event created using it:

https://github.com/kamilogorek/sentry-typescript-sourcemaps/
https://sentry.io/share/issue/9f1c1dc40e4a4118b637b5074a7f316f/

@daaain
Copy link
Author

daaain commented Jul 30, 2019

I cloned your repo and it does output separate source and map files.

image

It might still work, but in that case the docs are misleading claiming that:

The first one is configuring TypeScript compiler in a way, in which we’ll override sourceRoot and merge original sources with corresponding maps. The former is not required, but it’ll help Sentry display correct file paths, e.g. /lib/utils/helper.ts instead of a full one like /Users/Sentry/Projects/TSExample/lib/utils/helper.ts. You can skip this option if you’re fine with long names.

@kamilogorek
Copy link
Contributor

kamilogorek commented Jul 30, 2019

You are talking about this part?

merge original sources with corresponding maps

inlineSources does just that, unless I misunderstood you?

@daaain
Copy link
Author

daaain commented Jul 30, 2019

Yes, talking about that part. With my change proposed here you get only source files with the map inlined as Base64, with your current setting you get source and map files, and what inlineSources does is to put a link to the map files at the bottom of the source files, e.g. //# sourceMappingURL=bar.js.map.

I guess it's not a big difference as long as it works, but it did confuse me and I though that was the issue why it didn't work, but now I realise it's probably that I need to use the Sentry CLI to upload the sources, just doing curl https://sentry.io/api/hooks/release/builtin/... isn't enough.

@kamilogorek
Copy link
Contributor

it's probably that I need to use the Sentry CLI to upload the sources, just doing curl https://sentry.io/api/hooks/release/builtin/... isn't enough.

It can be it, but I'm not an expert in sentry-cli tbh. It does some sourcemaps rewrites for sure though.

Also regarding the change, it'd be detrimental to larger setups, where CI builds a project and sends artifacts to some other servers (eg. serverless instances), as it makes bundles quite large. And you don't need source maps on the serve once they are uploaded to Sentry.

@github-actions github-actions bot locked and limited conversation to collaborators Dec 16, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants