codymikol / karma-webpack

Karma webpack Middleware
MIT License
830 stars 222 forks source link

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

Closed alabbas-ali closed 6 years ago

alabbas-ali commented 6 years ago

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

Issues

SemVer

43081j commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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

michael-ciniawsky commented 6 years ago

Released in v3.0.5 🎉

alabbas-ali commented 6 years ago

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 https://github.com/webpack-contrib/karma-webpack/pull/360

edmorley commented 6 years ago

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

edmorley commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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

thijstriemstra commented 6 years ago

Can this be released?

fulv commented 5 years ago

Thank you, it works for me now! +1