Issue 104 - Add source map to NPM and PyPi packages #105
Conversation
| include dash_renderer/dash_renderer.dev.js | ||
| include dash_renderer/dash_renderer.dev.js.map | ||
| include dash_renderer/dash_renderer.min.js | ||
| include dash_renderer/dash_renderer.min.js.map |
There was a problem hiding this comment.
Adding map files, reordering
dash_renderer/__init__.py
Outdated
| 'https://unpkg.com/dash-renderer@{}' | ||
| '/dash_renderer/dash_renderer.min.js.map' | ||
| ).format(__version__), | ||
| 'namespace': 'dash_renderer' |
There was a problem hiding this comment.
Add maps to Dash
There was a problem hiding this comment.
Not sure it needs an entry here for the map, the _js_dist are files automatically served on the index. I think the js bundle will request the map on it's own.
There was a problem hiding this comment.
You're right that the JS file will request it on its own if the dev tools are opened -- but it will resolve it to its relative path, for example
There was a problem hiding this comment.
Or will Dash still allow the request to go through even if it's not defined here?
There was a problem hiding this comment.
In any case, I'll try it -- I assumed it was necessary :)
There was a problem hiding this comment.
Yes dash should allow it, before the map changes in dash I remember seeing 500 errors for map requests.
There was a problem hiding this comment.
Tested it out against the dash-table's examples and if the map is not present here, it doesn't get loaded. Dash probably tells the browser there is no such resource; and the browser continues on with its life?
There was a problem hiding this comment.
So it seems this additional entry is required
There was a problem hiding this comment.
Yes, but the 500 only happens if there is no extension support for map and the map is in the init file
There was a problem hiding this comment.
Ah your right just looked at the code and we check the if the path is registered so it needs to be in the dist for dash to allow it.
| @@ -1 +1 @@ | |||
| __version__ = '0.15.1' | |||
| __version__ = '0.15.2' | |||
There was a problem hiding this comment.
Bump version
| @@ -1,6 +1,6 @@ | |||
| { | |||
| "name": "dash-renderer", | |||
| "version": "0.15.0", | |||
| "version": "0.15.2", | |||
There was a problem hiding this comment.
Bump version.. for some reason this had not been bumped to 0.15.1 for NPM
| @@ -31,6 +31,7 @@ module.exports = (env, argv) => { | |||
| library: dashLibraryName, | |||
| libraryTarget: 'window', | |||
| }, | |||
| devtool: 'source-map', | |||
There was a problem hiding this comment.
Source map! This setting is slower but more accurate than other mappings. See here: https://webpack.js.org/configuration/devtool/
There was a problem hiding this comment.
Need to be sure I don't lose my debugging support with this, I remember the plain source-maps did not work so well.
There was a problem hiding this comment.
Is this the in-IDE support you mentioned to me a little while ago?
There was a problem hiding this comment.
Yes, I'll try it to be sure.
There was a problem hiding this comment.
Thanks. If it doesn't work I'll change it to:
mode === 'development' ? 'eval-source-map' : 'source-map'
which will give you the old behavior in dev
There was a problem hiding this comment.
🎉 Love it! Been wanting to get source maps in for a while but never got around to it. Approving it right away but let's make sure @T4rk1n's IDE support doesn't break.
|
Got a 500 error the first time I did a review, seems it posted it twice now. Whoops! |
|
If anyone wonders, the dev js file is smaller but removing the "eval source map" causes it to take 1 line per code line instead of squishing everything (with evals), explaining the 30k lines diff |
|
Getting a |
|
That's because we're not using the dynamic flag yet. When loaded "correctly" through the devtools this will not happen anymore. |
|
The external code map works for debugging, looked like it was even more stable for dev bundle. The min bundle also debugged but it was jumping over things. 💃 |
Thanks for checking this out. I thought we might be able to get rid of the dev build after the change but if you see improved behavior when debugging with it, let's keep it. |
Fixes #104
Relates to plotly/dash#478
Relates to plotly/dash-table#284
Relates to plotly/dash-core-components#404