fix react refresh sourcemap in dev mode #1963
Conversation
|
This pull request is being automatically deployed with Vercel (learn more). 🔍 Inspect: https://vercel.com/pikapkg/snowpack/evxmawgtg |
This comment has been minimized.
This comment has been minimized.
|
|
| @@ -0,0 +1,8 @@ | |||
| /** React Refresh: Connect **/ | |||
There was a problem hiding this comment.
nit: spelling... reatc in this file and the next should be react
| if (destBuildFile.code !== result) { | ||
| destBuildFile.map = undefined; | ||
| if (config.buildOptions.sourceMaps && (destExt === '.js' || destExt === '.css')) { | ||
| logger.warn(`source-map file not found in [${debugPath}]`, {name: step.name}); |
There was a problem hiding this comment.
Technically you could still have inline sourcemaps which wouldn't have an external map
| outputMap = await composeSourceMaps(filePath, destBuildFile.map, outputMap); | ||
| } | ||
| } else if (destExt === '.js' || destExt === '.css') { | ||
| logger.warn(`source-map file not found in [${debugPath}]`, {name: step.name}); |
There was a problem hiding this comment.
Same comment about inline sourcemaps
| outputMap = typeof map === 'object' ? JSON.stringify(map) : map; | ||
| if (destBuildFile.map) { | ||
| outputMap = await composeSourceMaps(filePath, destBuildFile.map, outputMap); |
There was a problem hiding this comment.
Do the sourcemaps need to be composed?
I would assume that that since the plugin has access to the sourcemap, it would potentially compose the maps.
|
Hey, just checking -- is this PR still alive? Really excited to try out Snowpack and TS sourcemap support is a blocker for me. Happy to pitch in if there's something I can do to get this over the finish line. |
|
@davidwallacejackson Sorry to reply so late. I'm still following this issue. There seems to be a lot of conflicts here. I will double check the conflicts a little later. |
|
Sorry @magic-akari, #2707 changed a lot of Snowpack internals. I'd actually love to help rebase this PR for you, if you don't mind waiting a little bit for me to make time. |
|
Okay, codebase has been refactored and is stable enough that rebasing this won't be a waste of time :) Appreciate everyones patience here. @magic-akari if I rebase this for you onto main, are you still interested in getting it merged? |
|
Yes. Please rebase it. |
|
Alright, I attempted a rebase here: https://github.com/snowpackjs/snowpack/tree/fix-sourcemap-dev-mode-react-refresh Unfortunately, quite a bit has changed. Normally git would be okay here, but the refactor included reorganizing files and moving logic around. This has made rebasing difficult. What I might suggest instead is attempting to re-create this PR on top of Sorry again for the delay here.No pressure if you don't have the time to recreate: we appreciate the contribution regardless. I'm going to close this PR for now, but feel free to create a new PR or just ping me in this branch if you're able to get this updated and I can take another look. |
Changes
This PR will fix react refresh plugin source maps in dev mode.
related issue: #1213
Testing
add new
reactRefreshOptionsconfig option and removemockBabelfunction.add
react-source-maptest filesDocs
bug fix only