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

alabbas-ali · 2018-09-14T09:32:29Z

When configuring Webpack to compile ts, the output is array and karma-webpack throw Exception like

08 09 2018 02:13:38.035:ERROR [karma]: 
TypeError: Path must be a string. Received [ 'spec/vendors.spec.js', 'spec/app.spec.js.map' ]
    at assertPath (path.js:28:11)
    at Object.join (path.js:1236:7)
    at Plugin.<anonymous> (../node_modules/karma-webpack/lib/karma-webpack.js:286:70)
    at Plugin.readFile (../node_modules/karma-webpack/lib/karma-webpack.js:305:5)
    at _combinedTickCallback (internal/process/next_tick.js:131:7)
    at process._tickCallback (internal/process/next_tick.js:180:9)

Type

Fix

Issues

SemVer

Patch

43081j · 2018-09-14T09:45:25Z

I do wonder if it'd make more sense to do the check when populating the outputs map.

If you can safely assume that the first entry is always the one we want, i would consider just adding the first one to the map and leave the existing code as is.

Are we sure that the first entry will always be right, too?

alabbas-ali · 2018-09-14T09:52:31Z

I do wonder if it'd make more sense to do the check when populating the outputs map.

If you can safely assume that the first entry is always the one we want, I would consider just adding the first one to the map and leave the existing code as is.

Are we sure that the first entry will always be right, too?

the first entry is always the js file, the second entry may be the map file or CSS file etc.. depending on the configuration of webpack. but the first entry is always the chank JS file. I think this is what we want to test. If I am not mistaken

michael-ciniawsky

👍 One clearification and two nits

michael-ciniawsky · 2018-09-14T13:08:06Z

src/karma-webpack.js

@@ -254,7 +258,11 @@ Plugin.prototype.readFile = function(file, callback) {
      })
    } else {
      try {
-        var fileContents = middleware.fileSystem.readFileSync(path.join(os.tmpdir(), '_karma_webpack_', this.outputs[file]))


var fileContents = '' if (condition) { fileContents = ... } else { fileContents = ... }

michael-ciniawsky · 2018-09-14T13:09:30Z

src/karma-webpack.js

@@ -237,7 +237,11 @@ Plugin.prototype.readFile = function(file, callback) {
  var doRead = function() {
    if (optionsCount > 1) {
      async.times(optionsCount, function(idx, callback) {
-        middleware.fileSystem.readFile(path.join(os.tmpdir(), '_karma_webpack_', String(idx), this.outputs[file]), callback)
+        if (Array.isArray(this.outputs[file])) {
+          middleware.fileSystem.readFile(path.join(os.tmpdir(), '_karma_webpack_', String(idx), this.outputs[file][0]), callback);


Is it ensured that this.outputs[file][0] points to the 'correct' file?

Did you try it it with e.g source maps enabled and/or extracting CSS ?

I tried it with both cases, it works very well :)

Alright :), I need to rely upon you here since this module has no test 😥

Sorry, It seems that it is not always the case for some webpack config :(
So i had to fix this in #360

michael-ciniawsky · 2018-09-14T13:10:55Z

src/karma-webpack.js

-        middleware.fileSystem.readFile(path.join(os.tmpdir(), '_karma_webpack_', String(idx), this.outputs[file]), callback)
+        if (Array.isArray(this.outputs[file])) {
+          middleware.fileSystem.readFile(path.join(os.tmpdir(), '_karma_webpack_', String(idx), this.outputs[file][0]), callback);
+        }else{


- }else{ + } else {

michael-ciniawsky · 2018-09-14T13:42:31Z

src/karma-webpack.js

@@ -254,7 +258,13 @@ Plugin.prototype.readFile = function(file, callback) {
      })
    } else {
      try {
-        var fileContents = middleware.fileSystem.readFileSync(path.join(os.tmpdir(), '_karma_webpack_', this.outputs[file]))
+
+		var fileContents = ''


Indent seems to be off here :)

michael-ciniawsky · 2018-09-14T13:44:36Z

@alabbas-ali Would you mind sending a PR with these changes formaster (v4.0.0-rc.2) aswell ? 🙏

michael-ciniawsky · 2018-09-14T13:55:22Z

src/karma-webpack.js

-
-		var fileContents = ''
+      try {	 
+	var fileContents = ''


Still off 😛

maybe my editor is off 🔢

michael-ciniawsky

@alabbas-ali Thx

michael-ciniawsky · 2018-09-14T14:40:47Z

Released in v3.0.5 🎉

alabbas-ali · 2018-09-17T16:01:40Z

I do wonder if it'd make more sense to do the check when populating the outputs map.
If you can safely assume that the first entry is always the one we want, I would consider just adding the first one to the map and leave the existing code as is.
Are we sure that the first entry will always be right, too?

the first entry is always the js file, the second entry may be the map file or CSS file etc.. depending on the configuration of webpack. but the first entry is always the chank JS file. I think this is what we want to test. If I am not mistaken

I did more testing, with different config, unfortunately, I found out that the assumption "that the first file is the JS" is not the case always, So I had to fix this please check out #360

edmorley · 2018-09-17T22:09:31Z

Could we also uplift this to the 4.x branch too?

edmorley · 2018-09-21T22:56:04Z

Could we also uplift this to the 4.x branch too?

Ping?

This is blocking us updating to 4.x, since otherwise we get:

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

alabbas-ali · 2018-09-21T23:20:49Z

Could we also uplift this to the 4.x branch too?

Ping?

This is blocking us updating to 4.x, since otherwise we get:
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

I create a pull request out of it #361. but please review #360 before.

fulv · 2018-09-28T15:15:40Z

Is this going to be included in the next 4.x release?

thijstriemstra · 2018-09-28T22:44:51Z

Can this be released?

fulv · 2018-11-30T19:15:39Z

Thank you, it works for me now! +1

🐛 fix TypeError: Path must be a string. Received Array

407d1fa

alabbas-ali mentioned this pull request Sep 14, 2018

Outputs can have multiple assets #355

Closed

michael-ciniawsky reviewed Sep 14, 2018

View reviewed changes

michael-ciniawsky added this to the 3.0.5 milestone Sep 14, 2018

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

michael-ciniawsky changed the title ~~🐛 fix TypeError: Path must be a string. Received Array~~ fix(karma-webpack): handle multiple outputs correctly Sep 14, 2018

michael-ciniawsky requested a review from ryanclark September 14, 2018 13:19

improve code structure

232731d

michael-ciniawsky approved these changes Sep 14, 2018

View reviewed changes

michael-ciniawsky added status: Approved and removed priority: 2 (urgent) severity: 3 (broken) status: Pending labels Sep 14, 2018

fix Indent spacing created be text editor

161ade7

michael-ciniawsky reviewed Sep 14, 2018

View reviewed changes

:) fix code editor off text shift

cb9912a

michael-ciniawsky approved these changes Sep 14, 2018

View reviewed changes

michael-ciniawsky merged commit 59de62c into codymikol:3.0.0 Sep 14, 2018

michael-ciniawsky removed this from the 3.0.5 milestone Sep 14, 2018

alabbas-ali deleted the multiple-output-files branch September 14, 2018 14:43

This was referenced Sep 14, 2018

Update 4 dependencies from npm JetBrains/ring-ui#406

Closed

Update 4 dependencies from npm JetBrains/ring-ui#407

Merged

dependencies bot mentioned this pull request Sep 15, 2018

Update 6 dependencies from npm JetBrains/ring-ui#409

Closed

alabbas-ali mentioned this pull request Sep 17, 2018

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

Merged

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

alabbas-ali commented Sep 14, 2018 •

edited by michael-ciniawsky

Loading

43081j commented Sep 14, 2018

alabbas-ali commented Sep 14, 2018

michael-ciniawsky left a comment

michael-ciniawsky Sep 14, 2018

michael-ciniawsky Sep 14, 2018

michael-ciniawsky Sep 14, 2018

alabbas-ali Sep 14, 2018

michael-ciniawsky Sep 14, 2018

alabbas-ali Sep 17, 2018

michael-ciniawsky Sep 14, 2018

michael-ciniawsky Sep 14, 2018

michael-ciniawsky commented Sep 14, 2018

michael-ciniawsky Sep 14, 2018

alabbas-ali Sep 14, 2018

michael-ciniawsky left a comment

michael-ciniawsky commented Sep 14, 2018

alabbas-ali commented Sep 17, 2018

edmorley commented Sep 17, 2018

edmorley commented Sep 21, 2018

alabbas-ali commented Sep 21, 2018 •

edited

Loading

fulv commented Sep 28, 2018

thijstriemstra commented Sep 28, 2018

fulv commented Nov 30, 2018

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

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

Conversation

alabbas-ali commented Sep 14, 2018 • edited by michael-ciniawsky Loading

Type

Issues

SemVer

43081j commented Sep 14, 2018

alabbas-ali commented Sep 14, 2018

michael-ciniawsky left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

michael-ciniawsky commented Sep 14, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

michael-ciniawsky left a comment

Choose a reason for hiding this comment

michael-ciniawsky commented Sep 14, 2018

alabbas-ali commented Sep 17, 2018

edmorley commented Sep 17, 2018

edmorley commented Sep 21, 2018

alabbas-ali commented Sep 21, 2018 • edited Loading

fulv commented Sep 28, 2018

thijstriemstra commented Sep 28, 2018

fulv commented Nov 30, 2018

alabbas-ali commented Sep 14, 2018 •

edited by michael-ciniawsky

Loading

alabbas-ali commented Sep 21, 2018 •

edited

Loading