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

alabbas-ali · 2018-09-21T23:53:18Z

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

…output

bdirito · 2018-10-10T22:39:57Z

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 · 2018-10-11T09:50:45Z

@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 · 2018-10-11T09:58:52Z

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 · 2018-10-11T16:47:45Z

@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 · 2018-10-17T18:03:40Z

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

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

ryanclark · 2018-10-17T18:06:19Z

src/karma-webpack.js

@@ -18,6 +18,18 @@ let isBlocked = false;

 const normalize = (file) => file.replace(/\\/g, '/');

+var getOutputPath = (outputPath) => {


Can we make this just a function instead? Or use const

Suggested change

var getOutputPath = (outputPath) => {

function getOutputPath(outputPath) {

Suggested change

var getOutputPath = (outputPath) => {

const getOutputPath = (outputPath) => {

ryanclark · 2018-10-17T18:06:48Z

src/karma-webpack.js

+      return outputPath[i]
+    }
+  }
+  return null


Missing semi-colon

Suggested change

return null

return null;

ryanclark · 2018-10-17T18:07:19Z

src/karma-webpack.js

-
-          this.outputs.set(entryPath, outputPath);
+
+          if (Array.isArray(outputPath)) 


Can you add braces around these statements please

ryanclark · 2018-10-17T18:08:31Z

src/karma-webpack.js

+      outputPath[i].indexOf(".js") !== -1 &&
+      outputPath[i].indexOf(".js.map") === -1
+    ) {
+      return outputPath[i]


Suggested change

return outputPath[i]

return outputPath[i];

bensampaio · 2018-10-26T08:29:00Z

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

plusgut · 2018-11-07T12:54:25Z

@michael-ciniawsky any updates?

thijstriemstra · 2018-11-07T13:05:32Z

Why is this not merged...

43081j · 2018-11-08T11:05:31Z

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 · 2018-11-15T23:26:11Z

@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 · 2018-11-20T10:42:13Z

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 · 2018-11-20T10:53:08Z

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 · 2018-11-20T16:32:31Z

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 · 2018-11-20T17:21:14Z

@matthieu-foucault temporary disable, need fix

matthieu-foucault · 2018-11-20T17:41:21Z

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

ryanclark · 2018-11-20T17:43:09Z

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

matthieu-foucault · 2018-11-20T17:46:48Z

@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 · 2018-11-20T17:47:59Z

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

uplift multiple output files fix to the 4.x branch

c432dd2

alabbas-ali mentioned this pull request Sep 21, 2018

fix(karma-webpack): handle multiple outputs correctly #357

Merged

2 tasks

michael-ciniawsky changed the title ~~uplift multiple output files fix to the 4.x branch~~ [v4.x] fix(karma-webpack): handle multiple outputs correctly Sep 22, 2018

michael-ciniawsky added this to the 4.0.0 milestone Sep 22, 2018

michael-ciniawsky added type: Bug severity: 2 (regression) priority: 2 (urgent) pr_action: Review semver: Patch status: Pending labels Sep 22, 2018

michael-ciniawsky requested review from michael-ciniawsky and ryanclark September 22, 2018 17:43

[v4.x] fix(karma-webpack): fix review comments, fix handling .js.map …

55d42c1

…output

thijstriemstra approved these changes Oct 17, 2018

View reviewed changes

ryanclark reviewed Oct 17, 2018

View reviewed changes

ryanclark requested changes Oct 17, 2018

View reviewed changes

v4 multiple output files, fix review comments

46963fa

thijstriemstra mentioned this pull request Oct 18, 2018

'The "path" argument must be of type string' since 4.0.0-rc.1 #363

Closed

edmorley mentioned this pull request Oct 20, 2018

Update dependency karma-webpack to v4.0.0-rc.3 neutrinojs/neutrino#1065

Merged

Removing linter warnings

e2f4cdc

Ivaylo-Lafchiev approved these changes Oct 24, 2018

View reviewed changes

thijstriemstra approved these changes Oct 24, 2018

View reviewed changes

ryanclark approved these changes Oct 24, 2018

View reviewed changes

edmorley mentioned this pull request Nov 5, 2018

Update dependency karma-webpack to v4.0.0-rc.3 mozilla/treeherder#4220

Merged

This comment has been minimized.

Sign in to view

ryanclark merged commit 41edac8 into codymikol:master Nov 20, 2018

alabbas-ali deleted the v4-multiple-output-files branch November 23, 2018 17:26

snyk-bot mentioned this pull request Apr 24, 2020

[Snyk] Upgrade karma-webpack from 4.0.0-rc.3 to 4.0.2 electrode-io/electrode#1625

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

alabbas-ali commented Sep 21, 2018 •

edited

bdirito commented Oct 10, 2018

mario-d-s commented Oct 11, 2018

alabbas-ali commented Oct 11, 2018

bdirito commented Oct 11, 2018

thijstriemstra commented Oct 17, 2018

ryanclark Oct 17, 2018

ryanclark Oct 17, 2018

ryanclark Oct 17, 2018

ryanclark Oct 17, 2018

bensampaio commented Oct 26, 2018

plusgut commented Nov 7, 2018

thijstriemstra commented Nov 7, 2018

43081j commented Nov 8, 2018

neemski commented Nov 15, 2018

edmorley commented Nov 20, 2018

alexander-akait commented Nov 20, 2018

matthieu-foucault commented Nov 20, 2018 •

edited

alexander-akait commented Nov 20, 2018 •

edited

matthieu-foucault commented Nov 20, 2018

ryanclark commented Nov 20, 2018

matthieu-foucault commented Nov 20, 2018

ryanclark commented Nov 20, 2018

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

		@@ -18,6 +18,18 @@ let isBlocked = false;

		const normalize = (file) => file.replace(/\\/g, '/');

		var getOutputPath = (outputPath) => {

	var getOutputPath = (outputPath) => {
	function getOutputPath(outputPath) {

	var getOutputPath = (outputPath) => {
	const getOutputPath = (outputPath) => {


		this.outputs.set(entryPath, outputPath);

		if (Array.isArray(outputPath))

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

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

Conversation

alabbas-ali commented Sep 21, 2018 • edited

Motivation / Use-Case

bdirito commented Oct 10, 2018

mario-d-s commented Oct 11, 2018

alabbas-ali commented Oct 11, 2018

bdirito commented Oct 11, 2018

thijstriemstra commented Oct 17, 2018

ryanclark Oct 17, 2018

Choose a reason for hiding this comment

ryanclark Oct 17, 2018

Choose a reason for hiding this comment

ryanclark Oct 17, 2018

Choose a reason for hiding this comment

ryanclark Oct 17, 2018

Choose a reason for hiding this comment

bensampaio commented Oct 26, 2018

plusgut commented Nov 7, 2018

thijstriemstra commented Nov 7, 2018

43081j commented Nov 8, 2018

neemski commented Nov 15, 2018

edmorley commented Nov 20, 2018

alexander-akait commented Nov 20, 2018

matthieu-foucault commented Nov 20, 2018 • edited

alexander-akait commented Nov 20, 2018 • edited

matthieu-foucault commented Nov 20, 2018

ryanclark commented Nov 20, 2018

matthieu-foucault commented Nov 20, 2018

ryanclark commented Nov 20, 2018

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

alabbas-ali commented Sep 21, 2018 •

edited

matthieu-foucault commented Nov 20, 2018 •

edited

alexander-akait commented Nov 20, 2018 •

edited