Skip to content
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

Investigate custom source map filenames in development #1322

Closed
gaearon opened this issue Dec 27, 2016 · 28 comments
Closed

Investigate custom source map filenames in development #1322

gaearon opened this issue Dec 27, 2016 · 28 comments

Comments

@gaearon
Copy link
Contributor

gaearon commented Dec 27, 2016

On mobile now but see this thread for context:

https://twitter.com/danbucholtz/status/813786323660210176

@bmeck
Copy link

bmeck commented Dec 28, 2016

xref: webpack/webpack#3603

@gaearon gaearon added this to the 0.10.0 milestone Feb 11, 2017
@gaearon
Copy link
Contributor Author

gaearon commented Feb 11, 2017

Tagging to revisit before 0.10.

@gaearon
Copy link
Contributor Author

gaearon commented Feb 15, 2017

@gaearon gaearon modified the milestones: 0.9.1, 0.10.0 Feb 15, 2017
@gaearon
Copy link
Contributor Author

gaearon commented Feb 15, 2017

Tagging for 0.9.1 because maybe the fix is super easy.

@Timer
Copy link
Contributor

Timer commented Feb 23, 2017

Should we consider this breaking? Do you think anyone is relying on the fact that it's webpack:///?
It could cause some confusion for people that regularly see that prefix in chrome dev tools or similar -- maybe a bad side effect for a patch update.

We could put it in 0.10.0 to be safe (I don't think it's too far out) ...

@gaearon
Copy link
Contributor Author

gaearon commented Feb 24, 2017

I'm cool with this being in 0.10.0.

@gaearon gaearon modified the milestones: 0.10.0, 0.9.1 Feb 24, 2017
@gaearon
Copy link
Contributor Author

gaearon commented Feb 24, 2017

We'll want to update these instructions if we change it: #1540
Also would be nice to investigate #1465 at the same time.

@Timer
Copy link
Contributor

Timer commented Mar 12, 2017

So I was fiddling with this... we can choose from (assets living in the build/ folder):

  1. ../packages/react-scripts/template/src/App.css
  2. /Users/joe/Documents/Development/OSS/create-react-app/packages/react-scripts/template/src/App.css
  3. packages/react-scripts/template/src/App.css

There's others, but these make the most sense.

@Timer
Copy link
Contributor

Timer commented Mar 12, 2017

Personally, I'm a fan of 1 or 2. They're the only true paths.
First is relative to build/, second is absolute. 😄

@gaearon
Copy link
Contributor Author

gaearon commented Mar 12, 2017

Do you mean for prod sourcemaps? Shouldn't we do this in development primarily for easy editing in DevTools?

@Timer
Copy link
Contributor

Timer commented Mar 12, 2017

These were the dev source maps. My mistake on basing out of build, even for production we should probably base out of appSrc.

@Timer
Copy link
Contributor

Timer commented Mar 13, 2017

Hmm.. looking at this more it actually seems like we need to base the dev ones out of build/static/js/ to have them appear "properly" in the chrome file tree. I don't think we can automagically get it mapped correctly so you don't have to tell chrome where the files live to allow editing in-browser.

Maybe we can cc someone from the chrome devtools team to ask how we can make this happen (probably won't because of security reasons). :)

This example is out of the appBuild folder. You can switch it to appPath and appSrc to see the effects:

diff --git a/packages/react-scripts/config/webpack.config.dev.js b/packages/react-scripts/config/webpack.config.dev.js
index c7948b6..8989a59 100644
--- a/packages/react-scripts/config/webpack.config.dev.js
+++ b/packages/react-scripts/config/webpack.config.dev.js
@@ -18,11 +18,7 @@ const InterpolateHtmlPlugin = require('react-dev-utils/InterpolateHtmlPlugin');
 const WatchMissingNodeModulesPlugin = require('react-dev-utils/WatchMissingNodeModulesPlugin');
 const getClientEnvironment = require('./env');
 const paths = require('./paths');
-
-// @remove-on-eject-begin
-// `path` is not used after eject - see https://github.com/facebookincubator/create-react-app/issues/1174
 const path = require('path');
-// @remove-on-eject-end
 
 // Webpack uses `publicPath` to determine where the app is being served from.
 // In development, we always serve from the root. This makes config easier.
@@ -77,6 +73,7 @@ module.exports = {
     filename: 'static/js/bundle.js',
     // This is the URL that app is served from. We use "/" in development.
     publicPath: publicPath,
+    devtoolModuleFilenameTemplate: info => path.relative(paths.appBuild, info.absoluteResourcePath),
   },
   resolve: {
     // This allows you to set a fallback for where Webpack should look for modules.

@gaearon
Copy link
Contributor Author

gaearon commented Mar 13, 2017

Hmm I feel silly but I don't understand what this would do. Why does it need the compiled files? If I run npm start for the first time, they don't exist, do they? How does Chrome know anything about where my build directory is, and why does it matter?

@Timer
Copy link
Contributor

Timer commented Mar 13, 2017

We tell the dev server to act like prod, see this snippit from dev config:

  output: {
    // Next line is not used in dev but WebpackDevServer crashes without it:
    path: paths.appBuild,
    // Add /* filename */ comments to generated require()s in the output.
    pathinfo: true,
    // This does not produce a real file. It's just the virtual path that is
    // served by WebpackDevServer in development. This is the JS bundle
    // containing code from all our entry points, and the Webpack runtime.
    filename: 'static/js/bundle.js',
    // This is the URL that app is served from. We use "/" in development.
    publicPath: publicPath,
  },

When running in dev mode, you can note that assets are being served from build/ (though not the ones on disk, they're "virtual"/in-memory).

@Timer
Copy link
Contributor

Timer commented Mar 17, 2017

@brandonmikeska did you still have interest in pursuing this the rest of the way?

@gaearon
Copy link
Contributor Author

gaearon commented Mar 17, 2017

I thought that line was not used at all (and only prevented crashes). How exactly does WDS use it? What does it mean to serve "from" a folder if it's serving from memory?

@Timer
Copy link
Contributor

Timer commented Mar 17, 2017

The majority happens here. Basically, webpack creates a bunch of express routes for our assets using the provided output path. e.g. /static/js/bundle.js, /static/assets/etc.ext.

Note that fs is inherited from the middleware, which, isn't a real filesystem. See here and here which detects webpack is in memory mode and creates a fake filesystem for express to use.

As far as express is concerned, it's serving static files.
As far as the file system they're coming from ... it's a fake fs impl in memory.

@gaearon
Copy link
Contributor Author

gaearon commented Mar 17, 2017

Basically, webpack creates a bunch of express routes for our assets using the provided output path. e.g. /static/js/bundle.js, /static/assets/etc.ext.

Right. But where does build figure in that? The paths are /static/... and don’t reference the build directory.

@Timer
Copy link
Contributor

Timer commented Mar 17, 2017

Oh, I meant to edit that build/ part away. At the time I was using the terminology "served from build/" to refer to the /static/{js,assets,css}/*.ext etc structure.

The root relative path should prob be appSrc.
This still doesn't place them in the correct location in chrome (to enable file editing in browser, easily... that is) -- not sure there's anything we can do about this.

@brandonmikeska
Copy link

@Timer yeah my apologies it was a busy week with releases and family in town. I was doing some research and everything I come across uses the build directory.

This blog shows a way, seems complicated to me, but a way to enable the file editing using chrome. https://medium.com/@rafaelideleon/webpack-your-chrome-devtools-workspaces-cb9cca8d50da#.8l5v85vzd

@Timer
Copy link
Contributor

Timer commented Apr 25, 2017

Added in 927c539.

@gaearon
Copy link
Contributor Author

gaearon commented May 11, 2017

Hmm. So how do we use this? I expected this would make it easy to edit files in Chrome, but I couldn't make it work.

@gaearon gaearon reopened this May 11, 2017
@Timer
Copy link
Contributor

Timer commented May 11, 2017

I'd assume we'd need to pass absolute paths...
I'm not sure how do make this work automatically without manually setting the file locations in chrome.

@gaearon
Copy link
Contributor Author

gaearon commented May 11, 2017

I'm okay with requiring to set locations, but I couldn't figure out how to do that either.. It just wouldn't set breakpoints for me.

@Timer
Copy link
Contributor

Timer commented May 14, 2017

@gaearon this might work after #2128

@gaearon
Copy link
Contributor Author

gaearon commented May 14, 2017

Hmm. I'm not sure how I did it but I think it works now. I assume this is the best we can do.

@gaearon gaearon closed this as completed May 14, 2017
@johnloy
Copy link

johnloy commented Feb 6, 2018

@gaearon @Timer

Hello! Sorry to be chiming in so long after this thread and 927c539, but I thought I'd point out a quirk I discovered resulting from the devtoolModuleFilenameTemplate addition.

I've been working with StackTrace.JS as part of an error handling utility I'm developing. StackTrace.JS uses source maps to correct filenames in Error stracktrace frames to their original source file, super useful for development and production error monitoring.

While testing StackTrace.JS's behavior in conjunction with CRA, I noticed that the absolute system paths it outputs as the sourcemap sources unsurprisingly result in absolute system paths in the "corrected" stack trace filenames. This is because there's just no way for the pinpoint() method in stacktrace-gps to translate an asset's system path into its server public path, if the information in sources is all it has.

When browser devtools log errors to the console, the filename portion of stacktraces are typically links that jump to a line:column location in the sources tab/inspector, but this only works if the filenames are either absolute or relative urls. Absolute system paths break this capability.

This isn't a huge deal, at least for my purposes, as I could theoretically map over the StackFrames to re-correct filenames to be root relative before an error gets logged to the console. Still, I wonder whether there's a devtoolModuleFilenameTemplate option that results in something that strikes a happy balance between easy-to-setup Chrome workspaces and normal linkable error stracktrace filenames.

If you don't mind, I'm going to investigate and report back, potentially with a PR.

Thanks for all your great work on CRA, btw!

@johnloy
Copy link

johnloy commented Feb 7, 2018

Ok. After playing with using devtoolModuleFilenameTemplate to produce sources paths that work well with stracktraces that link to the sources panel in browser devtools, I'm realizing that this totally breaks the reactErrorOverlay functionality to jump to source in your editor. The rationale for using absolute system paths in source map sources is a lot clearer to me now.

I also realized that that I was mistaken about how the jump-to-sources links work in the devtools console. If the sourcemap sources contains an entry for http://localhost:3000/Users/blahblah/myproject/src/index.js:162:0 and that's the url printed to the devtools console, it'll link to the source!

@lock lock bot locked and limited conversation to collaborators Jan 20, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants