codymikol / karma-webpack

Karma webpack Middleware
MIT License
829 stars 222 forks source link

Source maps broken since v5 #493

Closed squidfunk closed 10 months ago

squidfunk commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

@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 commented 3 years ago

@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 commented 1 year ago

@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 commented 1 year ago

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 commented 10 months ago

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 commented 7 months ago

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. 😿