Repository navigation
Replace --enable-source-maps with --disable-source-maps #38817
Description
Activity
Runtime source maps come with performance penalties. I can hardly believe it will be acceptable to enable source maps by default even in server production environments.
Makes sense. Maybe default to true in dev only then, by checking the
NODE_ENVenv var. We can keep the--enable-source-mapflag for production.Does Node itself currently use the
NODE_ENVto control anything? My impression was that this was merely a convention among package managers and packages, albeit a widely-used one. I think it would be weird to start checkingNODE_ENVwithin Node for this one thing.Reacted by Richard Lau, Antoine du Hamel, Chengzhong Wu and Lorenzo SchürchI can hardly believe it will be acceptable to enable source maps by default even in server production environments.
This could be argued either way as to whether proper stack traces should be captured or not by default (even in production environments). Presumably this only has any effect if a sourcemap exists, so it seems likely those who bother to ship sourcemaps to production would want them to apply (otherwise they could just not bother shipping them).
Runtime source maps come with performance penalties.
Really the thing that makes it questionable as to whether it could be default or not is when do these performance penalties actually apply.
- Does it significantly impact startup time?
- Does it cost when throwing an error or only when
.stackis read? - Does it have impacts on files that have source maps but never throw errors?
- Does it affect any other APIs other than
Error.prototype.stack/Error.captureStackTrace? - Does it affect files that don't declare a sourcemap?
In my opinion it would be fine to enable by default if it only applies a performance impact in cases where one would actually require it to, i.e. in files that use
//# sourceMappingURL, only when stacks are actually captured, and no performance impacts at other stages (e.g. during startup).Reacted by Rom, Damian Nowak, Lukas Elmer, Marvin Hagemeister, Edgar Ortega, Steven, Gerardo Lima and Michiel DralDoes it significantly impact startup time?
It can impact startup time. Node.js parses source-maps on loading the JavaScript sources so that would take times.
Does it cost when throwing an error or only when
.stackis read?It costs only when
.stackis read.Does it have impacts on files that have source maps but never throw errors?
Like said above, it can impact startup time if the source maps are unreasonably large.
Does it affect any other APIs other than
Error.prototype.stack/Error.captureStackTrace?Anything that printing errors will be affected as they are accessing
Error.prototype.stack, likeconsoleorutil.format(which is heavily used in common loggers).Does it affect files that don't declare a sourcemap?
Every line in
Error.prototype.stackwill be iterated to see if the frame is known to loaded source maps. So the whole program would take the performance penalties.it would be fine to enable by default if it only applies a performance impact in cases where one would actually require it to, i.e. in files that use
//# sourceMappingURLNo, errors thrown in any file will be penalized on accessing
error.stackeven if they themselves don't declare a source map.I cannot say that the impact is significant or not, as it may vary in different use cases. But small reduction can be accumulated to cause an observable slow down as source maps in node_modules must be counted.
However, I believe that a programming API that enables built-in source-maps support seems more reasonable to the OP's requirement. Vite or any developer tools can determine if they would like to enable source-map-support in that process at the very start of the program. In this way, there will be no need for users to set the
--enable-source-mapsflags for vite. Nor do we need to enable it by default in every environment.If a programming API sounds reasonable to you, I believe it will be a good start to work in that path.
Reacted by Rom, Michael Dawson, Sushant and Tommy D. RossiIf a programming API sounds reasonable to you, I believe it will be a good start to work in that path.
Yes, that sounds quite perfect actually.
So Vite will be able to decide whether source maps are applied? Meaning Vite could expose a flag
vite.config.js#disableSourceMapthat Vite users can use?Btw. is there a way to access the original stack trace? Source maps are often broken and not being able to access the exact source of error is extremely painful. When the source mapping is broken, it'd be wonderful to be able to dig into the original stack trace and do the mapping myself then having no clue where the error originates.
Btw. is there a way to access the original stack trace?
Sorry, but I didn't recall any means to do that without colliding the functionality provided by
--enable-source-mapsafter 88d9268 has landed.Source maps are often broken
May I have your elaboration on this? It will be helpful for us to resolve the problem if it could be fixed in Node.js core.
May I have your elaboration on this? It will be helpful for us to resolve the problem if it could be fixed in Node.js core.
I meant when the source map data itself is wrong, which is frequently the case.
That's why being able to access the original stack trace is important for these situations when the source mapping is wrong.
Debugging a broken stack trace is super painful.
Does it significantly impact startup time?
It can impact startup time. Node.js parses source-maps on loading the JavaScript sources so that would take times.
Does it impact startup time when there are no source maps?
(If not, then most existing code's startup time won't be affected, right?)
Anything that printing errors will be affected as they are accessing
Error.prototype.stack, likeconsoleorutil.format(which is heavily used in common loggers).In what situations would a developer/user reading
Error.prototype.stackwant speed instead of legibility?(As a devops person, I expect
Error.prototype.stackto be slow. I only use it after an unhandled error has been thrown. Are there other notable use cases?)However, I believe that a programming API that enables built-in source-maps support seems more reasonable to the OP's requirement.
Personally, I'm interested in the default. Moving to Typescript, I now gripe that I must add
--enable-source-mapsto every Docker image I ever create, forevermore. I expect virtually everyone using Typescript has the same issue.Reacted by Andy, Damian Nowak, Lukas Elmer, cobalt and TadjaurThis could be argued either way as to whether proper stack traces should be captured or not by default (even in production environments). Presumably this only has any effect if a sourcemap exists, so it seems likely those who bother to ship sourcemaps to production would want them to apply (otherwise they could just not bother shipping them).
This reasoning is convincing. Don't ship the sourcemaps, and they won't get used.
Source maps are often broken
May I have your elaboration on this? It will be helpful for us to resolve the problem if it could be fixed in Node.js core.
@legendecas I'm currently developing a website using Gatsby. I'm seeing an exception thrown like this:
at actions.createParentChildLink (/SNIP/node_modules/gatsby/src/redux/actions/public.js:1010:24)However, no file exists under this name. Only
node_modules/gatsby/dist/redux/actions/public.jsexists. And line 1010 is incorrect too. The source maps are probably usable for Gatsby developers who work on this code, and have created a link fromsrcto their working copy. And while useful in that particular situation, this source map is indeed broken. As an end user of the package, the stacktraces are meaningless for me, and make it more difficult for me to find the place to put a breakpoint for debugging.There has been no activity on this feature request for 5 months and it is unlikely to be implemented. It will be closed 6 months after the last non-automated comment.
For more information on how the project manages feature requests, please consult the feature request management document.
bad bot
Reacted by Tadjaur and Ankur PariharThere has been no activity on this feature request for 5 months and it is unlikely to be implemented. It will be closed 6 months after the last non-automated comment.
For more information on how the project manages feature requests, please consult the feature request management document.
bad bot
Reacted by Ankur PariharI'll close this then. Bumping is annoying and the consensus seems to be we're not going to enable this by default for performance reasons.
I think there is something wrong with
--enable-source-maps. Compared tosource-map-supportit just consumes so much time to even do aconsole.log. Can we limit the folders for source map lookup to save time, or is there another way to do this?
How about Node.js sets
--enable-source-mapsto true by default. If the user wants to get the real stack track, he can use--disable-source-maps.The problem here is that tools like Vite cannot really leverage
--enable-source-mapsbecause it forces Vite users to set the flag--enable-source-mapswhich is less than ideal. We don't want to tell Vite users "you need to use this and that flag"; we want things to just work. We are actually interested in using source map support (which is a lovely new feature!) but the flag thing is a problem, see related discussions at vitejs/vite#3300.Source map support is mostly taken care by frameworks; it is rarely implemented by the Node.js user himself. So we shouldn't leak this flag into user-land.
Also, in general, I'd argue that having source maps enabled by default seems to be a more sensible default.