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

Source maps broken since v5 #493

Closed
squidfunk opened this issue Feb 16, 2021 · 12 comments
Closed

Source maps broken since v5 #493

squidfunk opened this issue Feb 16, 2021 · 12 comments

Comments

@squidfunk
Copy link

  • Operating System: macOS
  • Node Version: 14.15.4
  • NPM Version: 6.14.11
  • webpack Version: 5.22.0
  • karma-webpack Version: 5.0.0

Expected Behavior

Source maps are mapped correctly

	Error: Oops!
	    at UserContext.<anonymous> (webpack://.../tests/test.spec.ts:6:9 <- test.spec.2488013834.js:8055:11)
	    at <Jasmine>

Actual Behavior

Source maps are not mapped at all

	Error: Oops!
	    at UserContext.<anonymous> (/var/folders/8z/8l6rfv895bgbst1hfmc19khh0000gn/T/_karma_webpack_269312/commons.js:7854:11)
	    at <Jasmine>

Code

I cannot provide the source, but adding karma-sourcemap-loader to this repository clearly shows the issue:
https://github.com/mcliment/typescript-karma-webpack-coverage-sample

How Do We Reproduce?

Check out the repository, install karma-sourcemap-loader, add it to preprocessor and see it in action.

Potential fix

I dug around in the code and found that commenting out these lines fixes the issue, i.e. source maps are correctly mapped as listed in the expected behavior:

https://github.com/ryanclark/karma-webpack/blob/ef7edb6b6756fb563871eaff88ca876892694896/lib/webpack/defaults.js#L17-L32

It looks like splitting out code into the runtime and commons chunk breaks source map support.

@squidfunk
Copy link
Author

squidfunk commented Feb 16, 2021

Temporary workaround:

config.set({
  webpack: {
    optimization: {
      splitChunks: {
        cacheGroups: {
          commons: {
            test: /\/node_modules\//,
            name: "commons",
            chunks: "all"
          }
        }
      }
    }
  }
})

This will only split out common dependencies into the chunk (thus they won't be source mapped), but keep source map support for code that is outside of node_modules.

@codymikol
Copy link
Owner

Thanks for opening an issue and looking into this!

I just ran tests after commenting out the default optimization config and they all continue to pass.

@daKmoR I noticed you added this initially, do you happen to remember if there were reasons beyond performance for setting these optimizations?

If just removing these defaults fixes sourcemaps and I'm not overlooking some edge case that could break by removing this, I can definitely do so and get that into the 5.1.0 release.

@daKmoR
Copy link
Collaborator

daKmoR commented Feb 17, 2021

iirc the only optimization that was needed was to share chunks across multiple test files.

I can't really remember if we had tests for that 🙈

but enabling source maps should not break that 🤔

More I don't know... 😅

@squidfunk
Copy link
Author

Inline source maps are still present in the commons.js bundle, but they are entirely stripped from the spec files. I agree that bundling common dependencies is better, especially for large test suites, but then somehow source maps should be preserved. I dug around Webpacks docs but I've found no possibility to preserve (or remap) sourcemaps after splitting chunks. Thus I guess disabling it might be the best approach.

FYI, I ended up using the much simpler, which seems more stable that what I've posted before:

config.set({
  webpack: {
    optimization: {
      splitChunks: false
    }
  }
})

@codymikol
Copy link
Owner

There should definitely be a straightforward was to enable source maps across the board, I might have some time next week to take a look at this.

@butchler
Copy link

We ran into this problem in our project when trying to upgrade to Webpack 5. I tried setting splitChunks: false as suggested, but that didn't work because many of our tests use async imports and need to be able to load chunks while running in the browser (and some tests even make assertions related to modules being lazy-loaded). I also tried this workaround but that didn't work with our tests either.

I was able to get it working by overriding the karma and webpack configuration to essentially subvert many of the things that karma-webpack tries to do:

{
  plugins: ['karma-webpack'],
  frameworks: ['webpack'],

  files: [
    {
      // allTests.spec.js uses Webpack's require.context() to include all other
      // *.spec.js files in the project, but using `pattern` to match all
      // *.spec.js files should work too.
      pattern: 'allTests.spec.js',
      watched: false,
      included: true,
    },
    {
      // Serve, but do not include, all chunks
      pattern: path.join('karma-webpack-build', 'chunks', '*.js'),
      watched: false,
      included: false,
      served: true,
    },
  ],

  preprocessors: {
    'allTests.spec.js':  ['webpack', 'sourcemap'],
  },

  proxies: {
    // Allow chunks to be loaded when tests run in the browser.
    '/bundles': '/base/karma-webpack-build',
  },

  webpack: {
    devtool: 'inline-source-map',
    output: {
      // Put chunks in separate folder so we can use the karma `files` config
      // to serve, but not include, all chunks.
      chunkFilename: 'chunks/[id].js',
      // Override output path and set public path to match proxy so chunks can be loaded.
      path: 'karma-webpack-build',
      publicPath: '/bundles',
    },
    optimization: {
      // Each of these options are intended to reset one of the settings from
      // `karma-webpack`'s default config back to Webpack's default settings for development mode.
      //
      // This is necessary to get source maps working (but I don't know why).
      runtimeChunk: false,
      splitChunks: {
        chunks: 'async',
        minSize: 10000,
        cacheGroups: {
          commons: {
            name: 'commons',
            // Use a test that always returns false, because we want to make
            // sure nothing ever gets put in this group.
            test: () => false,
          },
        },
      },
    },
  },
}

Using this setup means that the commons.js and runtime.js files that karma-webpack adds are unused. karma-webpack still preprocesses the entry file(s), but the Webpack optimization is reset so that chunks can be created, and the Webpack output paths and Karma proxy are configured so that the chunks can be loaded in the browser.

I understand that this is quite a large departure from the approach that karma-webpack currently uses, so I don't expect this to be the accepted solution to this problem, but hopefully this config helps someone else who's struggling to upgrade to Webpack 5.

@AprilArcus
Copy link
Collaborator

AprilArcus commented Aug 23, 2021

@codymikol this is impacting my team; our chunks are too large for karma to handle with splitChunks off. I can pick this up and run with it if you're tied down. Do we have any concerns about this being a breaking change?

@codymikol
Copy link
Owner

@AprilArcus If we have to make a breaking change we can, it would just mean a bump to the major version. I just bought a house so I'm pretty preoccupied at the moment, but I could certainly make time for a code review. Thanks for offering to take a stab at fixing this!

@devlinjunker
Copy link

@codymikol @AprilArcus was any progress made on this?

If not, do we have any idea of the changes necessary to fix sourcemaps for webpack v5?

@codymikol
Copy link
Owner

I am not currently working on a fix for this as it is not impacting my workflow, I would be happy to review a PR to do so though.

@codymikol
Copy link
Owner

As karma is now deprecated and coming up on EOL, we are no longer planning on any significant enhancements to this project and are instead going to focus on security updates, stability, and a migration path forward as karma's lifecycle comes to an end.

Thank you for supporting and using this project!

@dcsaszar
Copy link

Heads up: Because of #578 fixes by using optimization no longer work.

You may not want to update to https://github.com/codymikol/karma-webpack/releases/tag/v5.0.1 for this reason. 😿

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

No branches or pull requests

7 participants