-
Notifications
You must be signed in to change notification settings - Fork 220
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
Comments
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 |
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. |
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... 😅 |
Inline source maps are still present in the FYI, I ended up using the much simpler, which seems more stable that what I've posted before: config.set({
webpack: {
optimization: {
splitChunks: false
}
}
}) |
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. |
We ran into this problem in our project when trying to upgrade to Webpack 5. I tried setting I was able to get it working by overriding the karma and webpack configuration to essentially subvert many of the things that {
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 I understand that this is quite a large departure from the approach that |
@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? |
@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! |
@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? |
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. |
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! |
Heads up: Because of #578 fixes by using You may not want to update to https://github.com/codymikol/karma-webpack/releases/tag/v5.0.1 for this reason. 😿 |
Expected Behavior
Source maps are mapped correctly
Actual Behavior
Source maps are not mapped at all
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.
The text was updated successfully, but these errors were encountered: