fix(karma-webpack): handle `outputPath` correctly #360

alabbas-ali · 2018-09-17T16:00:53Z

This PR contains a:

Motivation / Use-Case

Refer to #357

The webpack output is not always the JS as first URL output

michael-ciniawsky

@alabbas-ali Thx for taking a stab at this again :)

michael-ciniawsky · 2018-09-22T17:46:01Z

src/karma-webpack.js

@@ -17,6 +17,15 @@ var isBlocked = false

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

+var getJsOutput = (outputPathArray) => {


- var getJsOutput + var getOutputPath

- ... = (outputPathArray) => + ... = (outputPath) =>

michael-ciniawsky · 2018-09-22T17:46:52Z

src/karma-webpack.js

@@ -17,6 +17,15 @@ var isBlocked = false

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

+var getJsOutput = (outputPathArray) => {
+  for (var _i = 0; _i < outputPathArray.length; _i++) {


- var _i + var i

michael-ciniawsky · 2018-09-22T17:48:39Z

src/karma-webpack.js

@@ -17,6 +17,15 @@ var isBlocked = false

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

+var getJsOutput = (outputPathArray) => {
+  for (var _i = 0; _i < outputPathArray.length; _i++) {
+    if (outputPathArray[_i].indexOf(".js") != -1) {


How des this work? Could you give a five examples please? What happens if e.g file.js.map takes precedence?

How des this work? Could you give a five examples please? What happens if e.g file.js.map
takes precedence?

You are right. I did not consider this case actually? I have in my webpack config 3 entries. like
entry: {
app: ${scriptsPath}/ibe/${IBEClient}/app.ts,
vendor: ${scriptsPath}/vendor.browser.ts,
polyfills: ${scriptsPath}/polyfills.browser.ts,
}
So, I move the entries to Karma config to get the same output form webpack. and remove it form webpack config. mostly I didn't see this case because is the .js output is always before file.js.map output in all my cases. The problem I faced is if the webpack shanks has .css.map as first output.

In any case i will add .js.map to if condition which should solve this case also :)

michael-ciniawsky · 2018-09-22T17:49:53Z

src/karma-webpack.js

-          this.outputs[entryPath] = outputPath
+          if (Array.isArray(outputPath))
+            outputPath = getJsOutput(outputPath)
+          if (outputPath != null)


michael-ciniawsky · 2018-09-22T17:53:32Z

@alabbas-ali In which case(s) is the .js file not the first one in the list? Could you eleborate a bit here please?

alabbas-ali · 2018-09-22T18:56:58Z

@alabbas-ali In which case(s) is the .js file not the first one in the list? Could you eleborate a bit here please?

As I said in the comment up. I have 3 entries:

entry: {
app: ${scriptsPath}/ibe/${IBEClient}/app.ts,
vendor: ${scriptsPath}/vendor.browser.ts,
polyfills: ${scriptsPath}/polyfills.browser.ts,
}

I move them to karma config, with devtool: 'inline-source-map' the output was the .js files as first output.
I had also .js.map and .css as output array for each chank.
But I also test it with devtool: 'source-map' which it was fine also a case.
Unfortunately, I had to at some point to change the order of the entire like:
entry: {
vendor: ${scriptsPath}/vendor.browser.ts,
polyfills: ${scriptsPath}/polyfills.browser.ts,
app: ${scriptsPath}/ibe/${IBEClient}/app.ts,
}
with devtool: 'source-map'
which made .css.map as first ouput for some reason.

dominykas · 2018-09-24T12:08:02Z

Using https://github.com/webpack-contrib/mini-css-extract-plugin also moves the CSS to be the first entry in the list.

alabbas-ali · 2018-10-10T12:08:36Z

Are you going to merge this in the near future ??

alabbas-ali · 2018-11-21T13:21:42Z

Could You merge this also ???

dominykas · 2018-11-23T08:25:41Z

Thanks for the merge! Anything left to do before this can be published on npm?

matthieu-foucault · 2018-11-23T16:21:14Z

It seems that the fix that we published in 4.0.0-rc.3 didn't fix things for everyone (see #355 ), so I'll investigate that before publishing

Lyrkan · 2019-03-11T16:03:13Z

@matthieu-foucault Hi!

We recently noticed this issue in Webpack Encore (see symfony/webpack-encore#256 (comment)) since we always use the mini-css-extract-plugin.

Is there any plan about releasing a new 3.x version with that fix? (it seems to work fine with 4.0.0-rc.6)

🐛 fix multiple output files, assamption fist file is JS is wrong

2c0715f

alabbas-ali mentioned this pull request Sep 17, 2018

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

Merged

2 tasks

alabbas-ali and others added 2 commits September 17, 2018 18:03

intent spacing

8b5a22d

remove unnecessary checks from code

7666ead

michael-ciniawsky suggested changes Sep 22, 2018

View reviewed changes

michael-ciniawsky changed the title ~~🐛 fix multiple output files, assumption fist file is JS is wrong~~ fix(karma-webpack): handle outputPath correctly Sep 22, 2018

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

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

michael-ciniawsky requested a review from ryanclark September 22, 2018 17:52

michael-ciniawsky added the pr_action: Review label Sep 22, 2018

fix(karma-webpack): fix review comments, fix handling .js.map output

3470e92

multiple output files, fix PR review comment :)

a3a22b2

This comment has been minimized.

Sign in to view

matthieu-foucault merged commit 6d6a69a into codymikol:3.0.0 Nov 22, 2018

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

Lyrkan mentioned this pull request Mar 11, 2019

How can we test our js components ? symfony/webpack-encore#256

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(karma-webpack): handle `outputPath` correctly #360

fix(karma-webpack): handle `outputPath` correctly #360

alabbas-ali commented Sep 17, 2018

michael-ciniawsky left a comment

michael-ciniawsky Sep 22, 2018

michael-ciniawsky Sep 22, 2018

michael-ciniawsky Sep 22, 2018

michael-ciniawsky Sep 22, 2018

alabbas-ali Sep 22, 2018

michael-ciniawsky Sep 22, 2018

michael-ciniawsky commented Sep 22, 2018

alabbas-ali commented Sep 22, 2018

dominykas commented Sep 24, 2018

alabbas-ali commented Oct 10, 2018

This comment has been minimized.

alabbas-ali commented Nov 21, 2018

dominykas commented Nov 23, 2018

matthieu-foucault commented Nov 23, 2018

Lyrkan commented Mar 11, 2019

		@@ -17,6 +17,15 @@ var isBlocked = false

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

		var getJsOutput = (outputPathArray) => {

fix(karma-webpack): handle outputPath correctly #360

fix(karma-webpack): handle outputPath correctly #360

Conversation

alabbas-ali commented Sep 17, 2018

Motivation / Use-Case

michael-ciniawsky left a comment

Choose a reason for hiding this comment

michael-ciniawsky Sep 22, 2018

Choose a reason for hiding this comment

michael-ciniawsky Sep 22, 2018

Choose a reason for hiding this comment

michael-ciniawsky Sep 22, 2018

Choose a reason for hiding this comment

michael-ciniawsky Sep 22, 2018

Choose a reason for hiding this comment

alabbas-ali Sep 22, 2018

Choose a reason for hiding this comment

michael-ciniawsky Sep 22, 2018

Choose a reason for hiding this comment

michael-ciniawsky commented Sep 22, 2018

alabbas-ali commented Sep 22, 2018

dominykas commented Sep 24, 2018

alabbas-ali commented Oct 10, 2018

This comment has been minimized.

alabbas-ali commented Nov 21, 2018

dominykas commented Nov 23, 2018

matthieu-foucault commented Nov 23, 2018

Lyrkan commented Mar 11, 2019

fix(karma-webpack): handle `outputPath` correctly #360

fix(karma-webpack): handle `outputPath` correctly #360