Open ciriousjoker opened 8 months ago
Been busy with other work lately, will look at this asap, sounds great though
This is a nice feature request. Some discussion topics on it:
I would like to propose that further additions are needed before we could release this feature:
build
output does indeed generate *.js.map
files in dist
project.json
schema, we should update the plugin migration tool to add these changes to existing plugin users workspace firebase projects.*.map
filesDiscuss if map files should be deployed or ignored If they are deployed, we'd need to document how plugin users can use them such as described here If not, we'd need to add ignore rules to function configs for *.map files
add my 5 cents here. The sourcemaps should be definitely deployed by default. Since code is bundled finding a real file causing an issue would be hard without sourcemaps. Default code generator should insert import 'source-map-support/register';
with a comment why it used instead of "--enable-source-maps" native node flag.
@thekip thanks for your comment. I broadly agree. One concern I have though is runtime performance for functions. source map files can be large, and loading them wont be cheap, do we know if they are only loaded when a crash occurs?
@thekip
...with a comment why it used instead of "--enable-source-maps" native node flag.
Doing a bit of research, it seems as though using node's --enable-source-maps
option would be the easiest implementation, since it brings native node support for source maps, and we could just define:
NODE_OPTIONS=–enable-source-maps
in our .env
or .env.local
file I believe. This wouldn't require any additional workspace dependencies for source-map-support
nor any wrangling with esbuild banners to import it at the top of the bundle. It might be good enough for most use cases.
However, there's a very interesting discussion here indicating that the node implementation is extremely slow with larger sourcemap files (like the ones we'd get with esbuild bundles), which seems to add risk of performance penalties in production cloud functions - not ideal given that CPU time is billed.
It does seem that performance costs for either node native or source-map-support
only seem to be incurred when an Error's stack trace is accessed, which is reassuring.
One contributor has benchmarked various source map solutions here, with node native support not faring too well in v16-v17, and also flagging incorrect stack traces which is not encouraging. There's no data in the benchmarks for v18+.
Some of these concerns may be more related to how big or minified the source map files are however, so on a related note, this comment is interesting also - using an esbuild
plugin to exclude source maps for bundled external deps - though to be fair, for Firebase functions generated with this plugin, we already exclude bundling of external dependencies.
I think I'm going to proceed carefully with this feature, and:
--inspect-functions
support in the serve
targetNODE_OPTIONS=–enable-source-maps
in the .env.local
fileThis at least sets a foundation for debugging functions locally.
We can then see what is the best way forward for supporting source maps in deployed functions.
I added this in our project by doing the following:
"sourcemap": true
to the esbuildOptions.--inspect-functions
to the emulators:start commandSample
launch.json