feat: use shared bundles

[daKmoR]

This PR contains a:

• new feature
• code refactor
• test update

Motivation / Use-Case

Refactor karma-webpack to create bundles not in isolation.
For the full story pls see this issue: #379

Breaking Changes

1. As this now uses a karma framework to add additional files this will need to be added everywhere.

// old:
frameworks: ['mocha'],

// new:
frameworks: ['mocha', 'webpack'],

2. The Alternative Usage described in the readme would be no longer needed.

Additional Info

I am aware that this is a huge refactor and while working on this feature I sort of already expected that it will end up with something like this. Again pls check the issue for how we ended up here.

This also removed the build step (don't think it's needed anymore) so we can easily test it as follows.

Howto test

easiest way:

# replace karma-webpack with forked version
npm i daKmoR/karma-webpack#feat/sharedBundles
# invoke as usual probably like this
npm run test

Possible ways to move forward

I see still some mandatory tasks:

• update readme
• cleanup dependencies (babel, async, ... no longer needed?)
• make a proper git history once everything is done

Some initially optional tasks:

• do proper testing (I will definitely follow up with an additional pull request if we say this is the way forward)

closes: #379

[jsf-clabot]

All committers have signed the CLA.

[matthieu-foucault]

@daKmoR FYI, this:

npm i karma-webpack
# replace karma-webpack with forked version
cd node_modules
rm -fr karma-webpack
git clone https://github.com/daKmoR/karma-webpack.git
cd karma-webpack
git checkout feat/sharedBundles
npm i

can be replaced by npm i daKmoR/karma-webpack#feat/sharedBundles

[matthieu-foucault]

Does this fix #22 as well?

[daKmoR]

npm i daKmoR/karma-webpack#feat/sharedBundles

uh nice I didn't knew :)

Does this fix #22 as well?

yes, CommonsChunkPlugin got "replaced/renamed" to splitChunks which is used in this implementation

[dogoku]

@daKmoR I'm beginning to test your branch out on our projects. I'll be commenting as I find issues.

Right off the bat, using Mac (Sierra), I had to create _karma_webpack_ in the temp folder myself, as karma would fail immediately and not run at all.

10 12 2018 09:23:08.289:ERROR [karma-server]: Server start failed on port 9876: Error: ENOENT: no such file or directory, open '/var/folders/1f/kdnyqc3n2t7f8l8kxcj0wk5w5t9pqt/T/_karma_webpack_/commons.js'

Once I did that, karma started running, but it seems like it's not respecting some of our custom path resolves, failing most of our tests. Still investigating if that's something caused by our setup as we are running karma through our own custom cli app, instead of directly from a project (think vue-cli), so it could be our fault.

[daKmoR]

@dogoku thx for testing it :)

I hope fix: make sure tmp folder exists solves the temp folder issue.

Also I didn't deep merge custom options yet - so every option you set on karmaconfig.webpack was not applied yet. That should work after fix: allow for deep nested webpack options.

=> this adds a dependency to merge - it was the first thing I found - if we have a different preference which lib to use for deep left merging just let me know.

[dogoku]

@daKmoR thanks for the update, I'll give it a go and let you know.

For merging configs, I'd suggest using https://www.npmjs.com/package/webpack-merge

It's a smarter version of merge that handles functions in a much better way. If however you feel that's an overkill, a deep merge like the one you are using might be enough as well

[dogoku]

I'm trying your latest change, but now webpack is complaining that the configuration object is not valid, in regards to the plugins.

I suspect what's happening is that the merge utility you are using is not handling the plugins correctly.
If possible, give webpack-merge mentioned above a try, which should handle plugins better.

---

Update 1: Using webpack-merge, I was able to proceed to the next step:

  updateWebpackOptions(newOptions) {
    // this.__webpackOptions = merge.recursive(
    //   true,
    //   this.__webpackOptions,
    //   newOptions
    // );
    this.__webpackOptions = webpackMerge(
      this.__webpackOptions,
      newOptions
    );
  }

I can now see everything compiling (and more importantly splitting), but Karma thinks that there are no tests to run. Will continue debugging and let you know.

---

Update 2: Finally, some good news :D

The issue of no tests being detected, was my code's issue. So with that aside, I am successfully running and passing over 80% of my tests.

[daKmoR]

Update 1: Using webpack-merge, I was able to proceed to the next step:

nice 👍 will change it later today 💪

I am successfully running and passing over 80% of my tests.

awesome 🎉
if you find anything else that needs change just let me know :)
also I am curious do you notice any difference in speed/performance?

[dogoku]

Alright, so a few more things:

Actual good news:

• Mocha's describe.only works across files (makes sense since we are bundling correctly now).

Potential good news:

• Maybe fixes karma-webpack does not support DefinePlugin constants #299 (I don't have the issue they are describing)
• I believe perf is increased, but as I'm still debugging, I haven't measured it.

Actual issues:

1) Passing a pattern that matches a single file to config.preprocessors, will cause karma to not recognise the tests (this is the issue I had above). E.g:

{
    files: [
      { pattern: 'test/unit/services/user-service/user-service-test.js' }
    ],
    preprocessors: {
      'test/unit/services/user-service/user-service-test.js': ['webpack']
    },
}

At the very least 2 files need to be matched by the patterns passed to preprocessor. E.g

{
    files: [
      { pattern: 'test/unit/services/user-service/user-service-test.js' }
    ],
    preprocessors: {
      // A glob that matches atleast 2 files
      'test/unit/**/*-test.js': ['webpack'],

      // OR 2 individual entries
      'test/unit/services/user-service/user-service-test.js': ['webpack'],
      'test/unit/services/data-service/data-service-test.js': ['webpack']
    },
}

2) All files matched by the patterns in config.preprocessors are bundled by webpack, regardless if we are not specifying them in config.files

From the 2nd example above, 'test/unit/**/*-test.js': ['webpack'] will cause webpack to create an entry for all my unit tests, even though I am only testing 1 file with karma { pattern: 'test/unit/services/user-service/user-service-test.js' }

Perhaps, we can cross-reference config.files and config.preprocessors to avoid creating entries for files that are not going to be used in the current test run. I imagine this is easier said than done, given Karma's file config is quite complex (http://karma-runner.github.io/3.0/config/files.html).

Potential issues

1) Watch mode was not waiting for all plugins to finish work and resulted in webpack compiling multiple times whenever I saved

This one is hard to explain, I need to spend more time debugging our custom plugins to make sure it's not them, before I can give you a clear case to work against.

2) Bundle sizes seem bigger than what they should, (some chunks are over 2MB)

I'm guessing it's babel-polyfill adding polyfills to each bundle, but again I need to confirm this one.

---

That's all for now. I'll update this post, as I have more details on each problem.

[matthieu-foucault]

Having an issue with a Typescript project.

The bug

In processFile(content, file, done) (karma-webpack.js:68), an exception is thrown because bundleContent is undefined.

The cause

path.parse(file.path).base would return spec.ts, while the keys of bundleContent are [ 'runtime.js', 'spec.js', 'vendors~spec.js' ]

Solution

It seems that using path.parse(file.path).name + '.js' fixes that part. Still having some errors further down the line

[daKmoR]

First, thank you for this awesome test feedback 💪
makes it really easy for me to pinpoint the problem and fix it 👍

Update 1: Using webpack-merge, I was able to proceed to the next step:

Use it via via 238744d

1. Passing a pattern that matches a single file to config.preprocessors, will cause karma to not recognise the tests (this is the issue I had above). E.g:

fixed via 8f08ba4

2. All files matched by the patterns in config.preprocessors are bundled by webpack, regardless if we are not specifying them in config.files

yeah seems I was a little eager with that one... turned the logic around... get all files first and then filter by preprocessor patterns via 34b2342

1. Watch mode was not waiting for all plugins to finish work and resulted in webpack compiling multiple times whenever I saved

which plugins are we talking about here? other karma preprocessors? or webpack plugins?

2. Bundle sizes seem bigger than what they should, (some chunks are over 2MB)

that seems weird - all the common parts (incl. babel polyfill should be within the commons.js) so commons.js should be big but each test file should only contain stuff unique to it

[daKmoR]

@matthieu-foucault
there should be no vendors~spec.js but that should be fixed with 8f08ba4

path.parse(file.path).base would return spec.ts

that could be "problematic" how do you do the typescript processing? is it a karma preprocessor or a separate build process?

[matthieu-foucault]

Seeing these warnings down the line: [middleware:karma]: Invalid file type, defaulting to js. ts
The browser refuses to execute the tests because of a mime type issue. I'll investigate later, and make a small typescript repo for easy testing.

[matthieu-foucault]

how do you do the typescript processing?

With a webpack loader (awesome-typescript-loader)

[daKmoR]

I'll investigate later, and make a small typescript repo for easy testing.

yes pls :) I don't have a typescript project using karma at hand 🙈

[dogoku]

@daKmoR the latest version fixed both problems I mentioned above, build seems considerably faster now, and since very few chunks are generated 👍

I am now trying to make the output of webpack less verbose, but so far it's ignoring any stats: 'errors-only' i pass to it.

[matthieu-foucault]

@daKmoR : here's a minimal example allowing you to test the issue with Typescript: https://github.com/matthieu-foucault/ts-karma-webpack-test

[daKmoR]

I am now trying to make the output of webpack less verbose, but so far it's ignoring any stats: 'errors-only' i pass to it.

via bf966f0 it's now mimicking the webpack-cli for stats so you can configure whatever you want 👍

What do you think would be good defaults?
that's it now

  stats: {
    modules: false,
    colors: true,
  },

@daKmoR : here's a minimal example allowing you to test the issue with Typescript:

thx will take a look 🤗

[daKmoR]

@daKmoR : here's a minimal example allowing you to test the issue with Typescript:

@matthieu-foucault via 5a05452 it now forces *.js files by default and also allows you to add your own via config.webpack.transformPath

solution inspired by karma-coffee-preprocessor

[dogoku]

it's now mimicking the webpack-cli for stats so you can configure whatever you want 👍

What do you think would be good defaults?

Personally, I like to turn off logging during tests, using stats: 'errors-only'. (dots is my fav reporter)
As long as we have the ability to configure, I think your suggested defaults are good enough.

[daKmoR]

Did we now reach a point where it makes sense to update this PR to become merge worthy (e.g. update Readme, clean up history, anything else?)

[dogoku]

I'd say it's a good enough beta.

We have managed to make all our tests green (the ones failing where not cleaning up properly).

Our stack includes: mocha, chai, chai-as-promised, sinon-chai, rewiremock (which requires a webpack plugin), custom webpack plugins (for localisation, config injection, etc). Being able to run all of that means it should handle most things we throw at it.

There's still a couple of things I want to test when in debug (watch) mode, but I have been busy with a different issue.

The question becomes, when can we release this, given v4 is still in RC and given the Typescript rewrite initiative

[matthieu-foucault]

I'd say it's a good enough beta.

I agree, not an RC yet

The question becomes, when can we release this, given v4 is still in RC and given the Typescript rewrite initiative

Whenever you want, it's a beta. v4 still has some bugs I would like to fix (#100, #337, #272), but I have no issue with v5 moving forward before v4 is released

[matthieu-foucault]

e.g. update Readme, clean up history, anything else

It will all be squashed into one commit, so don't worry about cleaning up history.
You'll need to undo the version change in package.json. standard-version will bump that for us, as well as write the changelog.

[daKmoR]

It will all be squashed into one commit, so don't worry about cleaning up history.

I feel more comfortable doing it myself 🙈
Added also all needed info for changelog to commit message

You'll need to undo the version change in package.json.

did that and updated readme - anything else?

=> if not it would be really nice to have a 5.0.0 alpha/beta/... release

[matthieu-foucault]

Looks good. I'll make an alpha release later today

[daKmoR]

did a small Readme change

Looks good. I'll make an alpha release later today

uh nice looking forward to it :)

[dogoku]

thanks both for your hard work <3

[matthieu-foucault]

Alright! It's available under the dev tag, so npm i -D karma-webpack@dev should do it.
Hopefully I can release v4 this week, and after that I think we should go with a blank slate, i.e. delete all old issues and focus on making v5 the best version of this project 😄

[daKmoR]

thx for the release :)

and that sound like a very awesome move forward :) count me in :)

[thescientist13]

Just wanted to stop by and say thank you for this work @daKmoR ! Was getting the same issue for testing web components and using the @dev version of this plugin fixed the issue. Thank you to you and the karma-webpack team! 🙌

[jquense]

greetings from 2020, does this approach work well from folks who have tried it? Wondering if it's better to switch to the forever-beta

[theo-staizen]

Yes, it has been working for us without any issues for a while now. It's been so long I forgot it was in beta still...
Having said that, we recently switched to a new stack, that doesnt involve karma, so I dont know whether this will still work for the upcoming webpack 5