You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
@mjackson just commented on PR #6823 that I submitted (and was merged) a few months ago. I'd like to reply, but that PR is locked to collaborators so I can't respond. Any chance someone cooler than me could unlock it so I could respond?
Here's Michael's comment and the response I was planning to write:
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.
First, some context: original source is 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 transpiled code. This won't be hard for experts like you, but for newbies that's a tall order.
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.
The text was updated successfully, but these errors were encountered:
@mjackson just commented on PR #6823 that I submitted (and was merged) a few months ago. I'd like to reply, but that PR is locked to collaborators so I can't respond. Any chance someone cooler than me could unlock it so I could respond?
Here's Michael's comment and the response I was planning to write:
First, some context: original source is 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:sourcesContent
inside the sourcemap, especially when combined with hot reloading.sourcesContent
. This means that if VS Code finds a sourcemap file, it will try to locate its original source files (from thesources
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 tosrc
to better clarify that the original source lives in that folder as opposed to the other top-level folders in each package.The text was updated successfully, but these errors were encountered: