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

[justingrant]

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:

[stale]

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.

[justingrant]

Not stale. Still waiting for maintainer feedback.

[pesimeao]

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

[alesdrobysh]

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

[ddc22]

PLease add this feature ":(

[stale]

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.

[zeevl]

Not stale. Please merge!!

…

On Sun, Apr 12, 2020 at 1:02 AM stale[bot] ***@***.***> wrote: 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. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#8227 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAML7UMKT4SNFNSTQZ5S5ZTRMFYQVANCNFSM4J63HDRA> .

[ch1ll0ut1]

Really need this...

[hawkaa]

I'd be interested in this functionality as well!

[JeremyGrieshop]

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]

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]

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

[eduardoborges]

Nice work!

[justingrant]

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! ;-)

[mrmckeb]

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

[Ryuurock]

Nice work!