build: add sourcemaps

[justingrant]

Unless your idea of fun is mentally un-transpiling ES5 code into ES6 and JSX, you'll like this PR. It makes debugging easier by adding sourcemaps for the react-router, react-router-dom, and react-router-config packages. The following build-time config changes are included:

1. Add sourceMaps: true to babel config for each package
2. Make a similar change to rollup output config, except it's called sourcemap: true here because, uh, diversity is good? ;-)
3. Added the source files in the modules folder to the files whitelist in each package's package.json, so that the original source will be installed by npm or yarn so that the sourcemaps have original source to point to.
4. Unlike the other module formats, UMD bundles all dependencies and therefore its sourcemaps include references to folders in node_modules. Unfortunately, the default paths to these folders are correct at build time but won't be correct when packages are installed on dev machines. This is a common problem, especially with monorepos. To fix, I added a one-line sourcemapPathTransform to rollup's output config (for UMD only). This transform replaces all variations of node_modules paths with a normalized path starting with ../../node_modules/ which is where these files are most likely to be installed by npm or yarn on a dev machine.

Note that this PR doesn't touch the react-router-native package because I am not familiar enough with React Native to know how to add sourcemaps there.

Using this PR, npm pack, and one of my apps that uses react-router, I verified that sourcemaps are now working with the VSCode and chrome devtools debuggers. "Working" means: (in case you want to verify it too)

• Original source is installed in my app's ./node_modules/react-router*/modules folders
• I can step from my code into react-router code and the original source is highlighted, not the transpiled source
• I can set a breakpoint on a line of original source in react-router or react-router-dom and the debugger will correctly break on that line when my app runs.

BTW, this PR is analogous to bvaughn/react-window#275 which I recently filed to add sourcemaps to Brian Vaughn's react-window library. That library also uses rollup and babel, so the changes needed to add sourcemaps there were similar to this PR. Slowly but surely I'm gonna try to fix sourcemaps of all my most important dependencies! ;-)

[timdorr]

Seems like a good idea to me.

[timdorr]

Good enough for me. Thanks!

[mjackson]

It's my understanding that we don't actually need to have the source code in our npm package, because the sourcemap already encodes that data, right? I'd like to avoid including our modules dir in our npm package if possible.

[pshrmn]

I believe that you are correct; as long as the sourcemaps have the sourcesContent property, the actual source files are unnecessary.

From the Source Map spec

Note: <url> maybe a data URI. Using a data URI along with “sourcesContent” allow for a completely self-contained source-map.

[mjackson]

Thanks for the thoughtful reply in #7016, @justingrant. Sorry, I should have unlocked this PR when I made my comment.

In that issue, one thing you said was:

When source files are available on disk, devs can set a breakpoint breakpoints in those files before the start of a debug session

That's not accurate, unless you're doing something that I haven't seen before. When you start up your app, you're not actually using any of our source code. You're using one of our builds. So how is one of those breakpoints going to work?

Turning it around, what are the reasons that original source should not be included?

I believe that including the original source in our npm package causes confusion, as has already been demonstrated here. You're not actually running that code, so setting breakpoints won't work, and changing the code won't change how your app runs.

Also, we reserve the right to do things in our source code that could probably break one of your tools, like including feature flags that we interpret at build time. One such flag that we currently use is the __DEV__ flag which is only true in development. If we include our source in our npm package, then people would be encouraged to try to actually use the raw source w/out building it first, which means they now need to know the details of our build system, which also isn't ideal.

[justingrant]

[copying my comment from #7016 here to keep discussion in one place, per @pshrmn request]

Thanks for unlocking! First, some context: with sourcemaps original source is usually present in two places: 1) in the sourcesContent prop of the sourcemap JSON file itself; 2) in the original source folder that's currently included in react-router packages in npm. In theory, you should only need one of those. In practice, publishing original source files to npm has multiple advantages:

• Can read & search original source outside of a debug session. Even without setting breakpoints or stepping through code in a debugger, it's really helpful to have dependencies' the original source on disk because it can be read, searched, copy/pasted, etc. without making a separate trip to GitHub and or locally cloning the repo. Another corollary of this is that IDEs like VSCode let you navigate by clicking on a breakpoint directly to the code where that breakpoint is set, but this won't work if a debug session isn't running unless the code file actually exists on disk.
• Easier to correlate with GitHub source, issues, PRs, etc. If the original source is on disk, a developer can easily correlate discussions and changes from the repo (e.g. a code snippet from an issue, or a particular line from a PR) with the source sitting in node_modules. For example, if a GitHub issue references a particular problematic line of code, or a recently-merged PR contains a potentially buggy line, a developer can easily find the same file/line on disk and set a breakpoint there.
• Breakpoints are easier for startup code. When source files are available on disk, devs can set a breakpoint breakpoints in those files before the start of a debug session and they'll be hit when the app runs. If the files aren't on disk, then a developer has to wait until the app is already running in order to set a breakpoint, or they need to somehow locate the correct transpiled code file (ESM? CJS? UMD? etc.) and then find the corresponding line in that transpiled code which can be hard, esp. for newbies.
• Breakpoints are less fragile. Although this is totally anecdotal without any good repro to back it up, I've generally found setting breakpoints in original source files to be more reliable (meaning they'll stick on the same line between debug sessions and be triggered reliably across different debug sessions) than setting breakpoints in the source that's reconstituted from sourcesContent inside the sourcemap, especially when combined with hot reloading.
• Prevents warnings from sourcemap-checking tools. Some tools, like https://github.com/Volune/source-map-loader/tree/fixes, will emit warnings if a sourcemap references a file that doesn't exist on disk. It's good to avoid these warnings.
• VSCode won't pick the wrong original source file. The VS Code debugger prefers files over the reconstituted source from sourcesContent. This means that if VS Code finds a sourcemap file, it will try to locate its original source files (from the sources array in the sourcemap). The problem is that if the relative URL in the sourcemap doesn't resolve the file, then VS Code will start hunting for other places it might be, assuming (correctly) that build pipelines sometime alter relative paths. The problem is that the pattern-matching algorithm it uses will sometimes be overeager and will match incorrect but similarly-named files. This is especially bad when the a file has a common name, e.g. ./index.js.

Anyway, that's a few reasons why including original source is preferred. None of these are super-high-priority things, but taken together they save developer time and reduce developer confusion when the original source is on disk. What do you think? Turning it around, what are the reasons that original source should not be included?

FWIW, one suggestion I'd have would be to rename the modules folder to src to better clarify that the original source lives in that folder as opposed to the other top-level folders in each package.

[justingrant]

Hi @mjackson - thanks for the quick reply! Here's a few notes:

When you start up your app, you're not actually using any of our source code. You're using one of our builds. So how is one of those breakpoints going to work?

This is the magic of sourcemaps! With a sourcemap, as long as your build config is correct, then you can set breakpoints (using VSCode or any file-based debugger, including AFAIK Chrome's workspace mode) in the original source code files and the debugger will halt there when the corresponding code is about to run in the transpiled JS. I'm not an expert in debugger implementation, but I believe it works something like this:

1. Whenever a new JS file is loaded in the browser (or node or...) the debugger looks for a source map comment at the end of the JS file. If found, it will pause execution of the file while the steps below happen. I believe that this is why you'll briefly see the "stopped at breakpoint" overlay in Chrome when starting a new debug session or when hot-reloading while debugging-- Chrome is pausing while it loads sourcemaps for the newly-loaded JS.
2. If a source map comment is present in that transpiled JS file, the debugger loads the corresponding .map file from disk into a JSON object in memory.
3. The debugger then resolves the relative URLs in sources into full paths of each original source file referenced in the sources array in the sourcemap JSON. If any of those paths correspond to files where breakpoints are set, then the debugger knows that these are "active" breakpoints that apply to this debug session. If you're using VSCode, you'll know a breakpoint is active because it's shown as a solid red circle when the debugger is running, as opposed to a hollow circle shown when the debugger hasn't matched that location to running code.
4. For each active breakpoint, the debugger needs to figure out where in the transpiled code (loaded in step 1 above) the breakpoint is. It does this by using the sourcemap's mappings array which maps the row/col in original source to row/col in transpiled source. (Or maybe vice versa, I forget which.)
5. Finally, the JS loaded in step 1 starts running in the debugger. Because all breakpoint locations were determined before the JS starts loading, breakpoints will work, even for startup code.
6. I suspect that this process is recursive so that if one sourcemap points to a source file that also has a sourcemap, this process continues.

Note that there are debugger settings which can control this behavior. For example, you can set disableOptimisticBPs in VSCode which I think controls whether the debugger will pause when loading a new file or will do all the steps above in parallel to the JS running. The latter behavior makes it faster to startup a debug session at the cost of being unable to debug the first few seconds of startup code. I think this comes up alot when trying to debug tests running under Jest.

changing the code won't change how your app runs.

Yep, but changing code inside node_modules is problematic even without sourcemaps. For example, react-router includes esm, cjs, and umd folders. How will a newbie developer know which one to change or even which one to set a breakpoint inside of? The answer is (at least in my case, as a relative newbie to the JS ecosystem) is that they don't know-- they'll experiment with all 3 (or 4 if there's a /modules) until finding one that works. Adding /modules may make this problem worse but it doesn't seem to introduce a brand-new problem.

Therefore, I think a better approach is to dissuade developers-- especially newbies who won't deeply understand transpilation, build systems, and bundle types-- from modifying node_modules code. Both for the reasons above, but also because npm update can unexpectedly blow away your changes. As a newbie I learned pretty quick that changing code inside node_modules was a really bad idea-- and FWIW I learned that lesson long before I understood the difference between ESM/CJS/UMD/XXX/CIA/FBI/etc.

I actually think that sourcemaps + original source can help prevent devs from modifying dependency code. AFAIK, the main reason that people modify node_modules dependency code is to insert console.log statements or other debugging code. If original source makes debuggers work better and more reliably, then there's no need to add logging to someone else's code.

My take is that we should be encouraging newbies (aka folks who won't know how to safely patch packages installed by npm or yarn) to use debuggers instead of modifying code that's not theirs, and that including sourcemaps + original source could help with this encouragement.

Also, we reserve the right to do things in our source code that could probably break one of your tools, like including feature flags that we interpret at build time. One such flag that we currently use is the DEV flag which is only true in development.

Yep, agreed. That said, now that this PR added the missing files and fixed your sourcemap config, I haven't seen any complaining from my sourcemap-validating tools. But if you change your build process to do a valid thing, like adding feature flags, that causes sourcemap-validating tools to complain, then IMHO the tools need to be fixed.. and if so they I'd volunteer to PR them! ;-)

If we include our source in our npm package, then people would be encouraged to try to actually use the raw source w/out building it first, which means they now need to know the details of our build system, which also isn't ideal.

I guess it depends what you mean by "use". If you mean "read, understand, copy, or debug" then I think these tasks are easier when original source is included in npm, as noted in my earlier post.

If you mean "modify in-place for reasons other than debugging" then I agree that including original source may make things more confusing. My assumption is simply that the former set of read-only tasks are much more common, especially for the set of relative newbies who may not yet understand that original source != source that actually runs.

In summary, I'm not implying that there are no downsides to including original source, only that the upsides IMHO seem to outweigh those downsides. What do you think?

BTW, sorry for the long response!