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

Compiler Option: sourceMap #59

Closed
jbrantly opened this issue Sep 25, 2015 · 12 comments
Closed

Compiler Option: sourceMap #59

jbrantly opened this issue Sep 25, 2015 · 12 comments

Comments

@jbrantly
Copy link
Member

The loader current supports the sourceMap option. However, it needs to be manually specified in the tsconfig.json. I think that people can be confused by this because they think they can just turn on sourcemaps in webpack and everything should work. See #17 for example.

Would it make sense to automatically turn on the TS sourceMap option if we detect that sourcemaps are turned on by webpack? I don't like overriding the user though, so maybe only do this is sourceMap is omitted from tsconfig.json. If the user explicitly set sourceMap: false then we would not override.

@blakeembrey
Copy link
Member

Personally, I think it's expected to be overridden (I don't need to know how as the consumer though) to "just work" as you would have enabled it in Webpack. I see it being a confusing case (and someone thinking it's a bug) if I have webpack's source maps enabled and they don't work.

@jbrantly
Copy link
Member Author

Alrightly! Sounds like this is something we should do then.

@basarat
Copy link
Member

basarat commented Sep 28, 2015

to "just work" as you would have enabled it in Webpack.

I agree. If webpack asks for sourcemaps, they should get sourcemaps 🌹 (irrespective of what they have in tsconfig.json)

@johnnyreilly
Copy link
Member

Definitely Definitely Definitely! The way I see it it's the same as how the files is (presumably) ignored in favour of what comes out of the require / import statements in code.

@jbrantly
Copy link
Member Author

@basarat @johnnyreilly What about the case where the user has explicitly set sourceMap: false in tsconfig.json but we detect that webpack wants sourcemaps?

@basarat
Copy link
Member

basarat commented Sep 30, 2015

we detect that webpack wants sourcemaps

Give webpack what it wants. Let sourcemaps be a webpack driven thing consistently. Less configuration options to explain.

They will not get sourceMaps unless webpack asks for it and if webpack asks for it they definitely get it.

@johnnyreilly
Copy link
Member

Agree with @basarat

@altano
Copy link

altano commented Nov 29, 2015

I agree source maps should just work by default for sure, but I'd like to politely strongly disagree on that last point. Sourcemaps are horrible to debug with. It's like trying to debug optimized CPP from source: it just doesn't work. My preferred way to use sourcemaps with webpack is to have the intermediate output be available in the debugger, not the original content.

In the case of ts-loader, having the original tsx file (for example) show up in Chrome is completely worthless. The highlighted line doesn't map to the line that is going to execute and it's impossible to step through the code. On the other end of the spectrum, not having sourcemaps and having to always debug the final bundle is equally annoying. The best thing to have be sourcemapped is the final JS output before bundling. So not the tsx file, but the produced jsx file with the React calls, before it is put into the bundle.

In short, I would love to be able to enable webpack source maps but disable ts-loader sourcemaps, so that I only get a source map from bundle.js to output of ts-loader, not the original files. And I would assume the best way of handling this would be to make it so that sourceMap: false in tsconfig.json turns of ts-loader source-map generation/merging.

@jbrantly
Copy link
Member Author

Thanks for your comments @altano. Actively overriding a user-specified option definitely doesn't sit right with me, and you've provided a use case for why.
.

@stale
Copy link

stale bot commented Jan 19, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

1 similar comment
@stale
Copy link

stale bot commented Mar 20, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix label Mar 20, 2019
@stale
Copy link

stale bot commented Mar 27, 2019

Closing as stale. Please reopen if you'd like to work on this further.

@stale stale bot closed this as completed Mar 27, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants