codymikol / karma-webpack

Karma webpack Middleware
MIT License
830 stars 222 forks source link

[v4.x] fix(karma-webpack): handle multiple outputs correctly #361

Closed alabbas-ali closed 5 years ago

alabbas-ali commented 6 years ago

This PR contains a:

Motivation / Use-Case

uplift fix multiple output files to 4.x branch

21 09 2018 23:54:23.585:ERROR [karma]: TypeError [ERR_INVALID_ARG_TYPE]: The "path" argument must be of type string. Received type object

bdirito commented 6 years ago

miniCssExtractPlugin breaks karma-webpack. Since i dont require styles one way or the other for my unit tests I just remove it from the webpack entry of karma.config as a workarround.

MiniCssExtractPlugin = require 'mini-css-extract-plugin'
webpackConfig = require './webpack.config'

removeMiniCssExtract = (conf) ->
    for rule in conf.module.rules
        if rule.use
            rule.use = rule.use.filter( (u) -> u != MiniCssExtractPlugin.loader)
    return conf

# ...
 webpack: removeMiniCssExtract(webpackConfig)
mario-d-s commented 6 years ago

@bdirito tried it but it didn't work... Karma-webpack is still receiving multiple entry points. What syntax is that code by the way?

Besides the mini CSS extract plugin, enabling source maps also results in multiple entry points being passed (.js and .js.map), but maybe I'm doing something wrong here...

alabbas-ali commented 6 years ago

I can't see the need to have such work workarround. if merging this branch will solve the problem. I test it also with MiniCssExtractPlugin and it is working fine.

bdirito commented 6 years ago

@mario-d-s MiniCssExtractPlugin is only one example of a webpack plugin that produces this issue. Various other plugins can and will also trigger this issue so it will not work in cases where you are using those. Depending on what those plugins do it may or may not be possible to remove them in the same manner, I can only do this because css is irrelevant for my unit testing. The syntax is coffeescript.

@alabbas-ali I would prefer to have a fix merged in to webpack-karma but in the absence of that happening (why is review taking so long for an 'urgent regression'?) I wanted something that didnt require building before use. Ideally this issue would be fixed and any such workarounds can be removed.

thijstriemstra commented 6 years ago

(why is review taking so long for an 'urgent regression'?)

I'd also like to know. Can this please be merged and released?

bensampaio commented 6 years ago

Any updates on this? Would be really great to have this merged so that I can go forward with upgrading to Webpack 4.

plusgut commented 5 years ago

@michael-ciniawsky any updates?

thijstriemstra commented 5 years ago

Why is this not merged...

43081j commented 5 years ago

I'm seeing a lot of issues in both 3.x and 4.x around sourcemaps and such too.

3.x has a fix in place for multiple outputs but it introduces more issues than it resolves. It means things like sourcemaps are lost/broken as their outputs are thrown away.

This may resolve the issue. If it does, it needs to be added to 3.x too if thats still being maintained.

neemski commented 5 years ago

@rynclark @michael-ciniawsky any updates on this? As many have pointed out, we are blocked from upgrading to Webpack 4 until this is merged/released

edmorley commented 5 years ago

Please can we review/merge this?

At this point just switching wholesale to Jest is seeming very appealing. The Karma ecosystem seems to be in decline.

alexander-akait commented 5 years ago

We need contributor for this loader, there is no one here right now, so if you can and have time for this ping me in gitter (https://gitter.im/webpack/webpack)

matthieu-foucault commented 5 years ago

I just got collaborator access. Bear with me as I am trying to get this merged. Currently, I cannot do that as the TravisCI build is pending (and will stay that way). It seems that this check has been ignored since #344 (at least) and that admin rights have been used to merge PRs.

If an admin sees this, can you please disable the required check?

alexander-akait commented 5 years ago

@matthieu-foucault temporary disable, need fix

matthieu-foucault commented 5 years ago

Well, I still can't merge this. Looks like only an admin can merge to master.

ryanclark commented 5 years ago

Yeah, that's why I didn't merge - we need @michael-ciniawsky as far as I'm aware

matthieu-foucault commented 5 years ago

@evilebottnawi seems to be able to change these things. Can you add @rynclark (my help isn't needed if Ryan is available) to the list of contributors allowed to push to master?

ryanclark commented 5 years ago

Thanks @matthieu-foucault. I'm also not able to publish to npm, though. // @evilebottnawi

alexander-akait commented 5 years ago

@rynclark WIP

alexander-akait commented 5 years ago

@rynclark send me you npm nickname in gitter https://gitter.im/webpack/webpack

alexander-akait commented 5 years ago

@rynclark Can you merge now?