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

Add source-map-loader for debugging into original source of node_modules libraries that contain sourcemaps #8227

Merged
merged 1 commit into from
Jun 2, 2021

Conversation

justingrant
Copy link
Contributor

@justingrant justingrant commented Dec 24, 2019

This PR is the next step to improve debugging of dependency (node_modules) code when running CRA apps in a debugger like Chrome or VSCode. This PR continues my previous PR, #7022, that changed babel config to ensure that sourcemaps would be generated properly for dependency libraries, including react itself.

However, what's still broken is the case where node_modules libraries include sourcemaps. This is increasingly common as more libraries are written in typescript and/or use features like async/await that make it very difficult to debug transpiled code. When debugging current CRA apps, you can't step into or set breakpoints in original (ES6, typescript, etc.) library source code. You can only debug transpiled code which can be very challenging for async-heavy or TS code.

To enable debugging the original source of node_modules dependencies, an additional step is needed: a webpack loader (source-map-loader) that extracts sourcemaps from node_modules dependencies and hands those sourcemaps off to webpack so that webpack can bundle those sourcemaps into the node_modules chunk that's emitted by npm start or npm build.

This PR adds this source-map-loader into CRA's webpack config between the lint module rule and the babel/css/etc rule below it. It fixes #8071, fixes #6596, and fixes #6994. Here's the config change:

        // Handle node_modules packages that contain sourcemaps
        shouldUseSourceMap && {
          enforce: 'pre',
          exclude: /@babel(?:\/|\\{1,2})runtime/,
          test: /\.(js|mjs|jsx|ts|tsx|css)$/,
          use: 'source-map-loader',
        },

I'm assuming that because this loader is added so early in the chain, therefore it's not relevant to the CSS-related loader-order issues described in #8281.

UPDATE 2020-11-29: Unlike all other warnings reported through webpack, this PR prevents warnings from source-map-loader from causing CI failures, because the main use case of sourcemaps is for interactive debugging and there's no interactive debugging possible in a CI environment. Therefore, it doesn't make sense to fail CI builds for warnings that the developer can't fix (because they're in a dependency not under the developer's control) and that won't affect CI success. Sourcemap warnings will still show up in the CI console output, but they won't cause the build process to exit with an error code.

If you want to see the warnings generated by dependencies with missing or invalid sourcemaps, here's code that requires a package whose sourcemaps currently point to source files that were not published to npm:

import { SaxesParser } from 'saxes';
new SaxesParser();

If you add this code to any CRA project, and run npm start or npm build, then you'll see console warnings like this:

image

@stale
Copy link

stale bot commented Jan 27, 2020

This pull request has been automatically marked as stale because it has not had any recent activity. It will be closed in 5 days if no further activity occurs.

@stale stale bot added the stale label Jan 27, 2020
@justingrant
Copy link
Contributor Author

Not stale. Still waiting for maintainer feedback.

@stale stale bot removed the stale label Jan 27, 2020
@pesimeao
Copy link

pesimeao commented Jan 28, 2020

I can't wait for this to be merged ;) thank you @justingrant

@justingrant justingrant changed the title WIP: add source-map-loader for debugging into original source of node_modules libraries that contain sourcemaps Add source-map-loader for debugging into original source of node_modules libraries that contain sourcemaps Feb 14, 2020
@alesdrobysh
Copy link

any progress here? It's been opened for 2 months

@ddc22
Copy link

ddc22 commented Mar 13, 2020

PLease add this feature ":(

@stale
Copy link

stale bot commented Apr 12, 2020

This pull request has been automatically marked as stale because it has not had any recent activity. It will be closed in 5 days if no further activity occurs.

@stale stale bot added the stale label Apr 12, 2020
@zeevl
Copy link

zeevl commented Apr 12, 2020 via email

@stale stale bot removed the stale label Apr 12, 2020
@ch1ll0ut1
Copy link

Really need this...

@hawkaa
Copy link

hawkaa commented May 11, 2020

I'd be interested in this functionality as well!

@JeremyGrieshop
Copy link

I recently used this feature via customize-cra and just remembered how awesome it is to finally have breakpoints and step through code over some very common and complicated module I wrote a while back. This request is so useful!

@justingrant
Copy link
Contributor Author

justingrant commented Jun 25, 2020

source-map-loader was finally refreshed with the fixes that this PR needed! I resolved the other open issues too. This PR should now be ready for merging.

@MasterCassim
Copy link

Would be nice to have this merged. Tested this locally and works perfectly.

@eduardoborges
Copy link

Nice work!

@justingrant
Copy link
Contributor Author

Hi @iansu, @mrmckeb, @ianschmitz, @petetnt - This PR has the 4th-highest number of upvotes in the CRA repo. It's a tiny (<10 line) change but hasn't gotten any maintainer feedback since I opened it 7 months ago. Any chance that one of you could review it?

Thanks so much for maintaining this project and for putting up with pesky contributors like me! ;-)

image

@mrmckeb
Copy link
Contributor

mrmckeb commented Jul 31, 2020

Hi @justingrant, are you able to share some any information around the performance impacts of this change?

@Ryuurock
Copy link

Nice work!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet