-
Notifications
You must be signed in to change notification settings - Fork 17
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
Inline sourcesContent
vs. sources
files: which should maintainers use?
#42
Comments
This is somewhat in line with one of the goals listed in #22
By achieving the goal above, it'd make using |
We touched on this briefly in one of the recent meetings and I think different folks have different wishes here. While I think we also hate large JSON files, we still prefer large JSON files over accidentally loading the wrong things. |
During iterative development at least, |
Yes, there is a big difference between local development / browser debug tools and the situation you have in a post-hoc tool like Sentry. |
I wonder if there could be some simplification where we recommend that packages be published one way (either as One thing I've observed consistently is that
The core problem is that any time two different files need to be in sync, it dramatically increases the chance of failure. The more atomic sourcemaps are, the harder it is to screw up. Sometimes I wonder if the best solution solution might be if all debug info were in a single file (e.g. |
These workflows (go-to-definitions and breakpoints) are actually really important IMO. Worth noting that virtual files increases complexity not only for VS Code but for all IDEs. Rehydrating the original physical files as per #42 (comment) is a neat solution though. |
Another reason to avoid In my experience however, if browsers are able to load a source map then they will often assume that files in Personally I would prefer to see browsers behave better in this situation, as requiring source maps to be unreachable if the sources are unreachable means that (depending on what exactly the server returns) you may get a warning in the devtools for every unreachable source map. |
This is mostly why I hope we can get |
IMO it's helpful to clearly differentiate two different sourcemap use cases:
I think you're talking about app sourcemaps only, right? IMO, enabling all OSS libraries to publicly expose their original sources in an IDE-friendly way should be a top goal of this working group, because debugging an app becomes vastly harder if the only way to see actual library sources is to clone the repo from GitHub and manually try to associate the transpiled and/or minified code being debugged with the sources from GitHub. I have to do that frequently, and it's horribly inefficient.
My understanding of sourcemap generation is that it has two logical parts: first generating sourcemaps for an app's source code, and then combining that sourcemap with sourcemaps for all libraries used by the app. I'm not sure if bundlers actually do it this way, but at least this is how I think about it without thinking about implementation details. If sourcemap-generating options (like blanking comments and/or obscuring variable names) are something that build tools want to support, then I think it's probably helpful to scope those options to the first, per-package step above (generating sourcemaps from original sources) and not globally at the "merge sourcemaps together" step, because it seems quite helpful for technically-adept users to debug into and report bugs about problems in apps (or in OSS libraries used by those apps) even when those apps' owners don't want to show their (non-OSS-library) proprietary sources to the world. Note that |
Personally I think these two problems are the same. In either case I would like to disassociate the downloading of the source maps from the built artifacts (see also #41 (comment)).
That I agree with in principle, but I'm not convinced that the solution will be publicly hosting them identified by a URL. But that I believe is a conversation for the other thread.
|
It's also reasonable for sourcemaps to look different in each case (not that I'm necessarily advocating for), since the bundler will transform the library sourcemap into an app sourcemap. So sources in dev can become sourcesContent in prod. Actually this may be the way many people are doing right now. |
I'd like to walk back my earlier comment as your comments made me realize that once the source map itself is private, it doesn't matter whether it links to similarly private sources or contains them. Similarly, a source map with inaccessible sources is not useful because there's nothing to map onto. I think the reason I wanted them separate initially is that usually, CLI tools that generate source maps place them in the same path as the files that use them, which means you can't make them private with security on a directory level (which is what I'm limited to currently). But because being unable to load the sources referenced by the source map seemed to break the devtools of various browsers, I moved the source maps to a different path with the same security as the source itself. So I no longer have a fundamental concern about |
This seems like a good goal, but I think the right path to achieve it may be different for the app case vs. the OSS library case. In the OSS library case, there's an npm CLI involved in both the publishing and downloading (aka installing) ends. So I could imagine a solution that adds options to I've been trying to get OSS maintainers to fix their sourcemaps for 4+ years, and it's an uphill battle. Any solution that is implemented in the platform pieces (npm, browsers, IDEs, etc.) will deliver value to developers many (for some packages, infinite) years quicker than something that requires maintainers to change anything. (npm isn't the only package manager or package repo, but IMO the same points above apply to the small number of other package managers and repos: it's straightforward to change the installer to either omit sourcemaps or omit non-sourcemap files from a download.) For apps (and private libraries), a few things are true that are different than for OSS libraries:
For the app case, it seems like the right place to do the decoupling would be in a build step and/or bundler plugin. I could even see sourcemap consumers like Sentry taking the lead in creating and maintaining such a tool if it combined stripping sourcemaps from bundles and submitting those sourcemaps to a server like Sentry's in a single CLI tool. I guess it's possible for the same build step or plugin to send both sourcemap-stripped code and sourcemaps to npm too, but at that point npm has to deal with sourcemaps getting out of sync to the package, being built with different source, etc. Those problems wouldn't exist if npm was doing the splitting on the server API handler. So it seems unlikely that npm would want to offer a publish API or CLI that accepted two separate code vs. sourcemaps files as inputs. Also, for legacy consumers of npm packages who don't know about splitting, the full package (including sourcemaps) still needs to be available somehow via |
There are two ways to expose source code with sourcemaps: either the code can be in
sourcesContent
or maintainers can ship their source files that are pointed to bysources
. (Hopefully not both, although I don't think both is illegal in the spec, which may be a mistake.)Many maintainers don't like shipping the files in
sources
, because it generates support issues for them. Here's a few of them, excerpted from this issue:import { foo } from 'some-package/src/bar.ts';
and when I change my TS code, e.g. to upgrade to a new TS version, they'll file bugs and complain that I broke their app even though they're not using my public API.import
orrequire
.However,
sourcesContent
also has problems:sourcesContent
as a second-class citizen. Ifsources
files are not present, several VS Code workflows don't work well, or not at all.sourcesContent
isn't really helpful.Should the new sourcemaps spec have an opinion about whether
sourcesContent
orsources
are the best option for maintainers? Would it simplify things a lot for the ecosystem if there were just one recommended solution?The text was updated successfully, but these errors were encountered: