-
Notifications
You must be signed in to change notification settings - Fork 220
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Bundles are created in isolation (not suitable for webcomponents) #379
Comments
Just adding my 2 cents. We were also facing the same issue when switching from browserify to webpack and we were surprised when we noticed this behaviour. In the end we opted for using karma-iframes, which runs each bundle in an isolated iframe. It works as expected and it's actually a clever solution to this and other problems, but it's not "ideal" for a few reasons:
While I understand it's not exactly apples-to-apples, karma-browserify solved the same problem for browserify, by creating a single bundle and then importing all the matched test files via stub layer, ensuring that everything is called once. See https://github.com/nikku/karma-browserify#how-it-works |
@dogoku thxxxx :) that sounds like a good workaround :) If we can't agree on a solution via So the question is could this "Multi Page Application Solution" work? |
😍 This project badly needs maintainers, so that's great if you can help! I'd say option 2 is the way to go, as it mirrors Karma's behavior. In terms of hints... Well I don't have any, I've mainly been putting out fires here, and haven't had time to take an in-depth look into the project 😅 Maybe @rynclark , @michael-ciniawsky or @d3viant0ne could quickly chime in 👋 |
I will post my progress here so if I am straying off just let me know. First I wanted to verify what an actual working webpack config should look. So that should be fine entry: {
one: './foo-one.js',
two: './foo-two.js',
},
output: {
filename: '[name].bundle.js',
},
optimization: {
splitChunks: {
chunks: 'all',
minSize: 0,
cacheGroups: {
commons: {
name: 'commons',
chunks: 'initial',
minChunks: 2,
},
},
},
}, Exact config used: https://github.com/daKmoR/karma-webpack-wc-bug/blob/webpackconfig/webpack.config.js Output will be 3 files
So that is exactly what we need - everything that is used more then once get's pushed to a shared See here the actual files: https://github.com/daKmoR/karma-webpack-wc-bug/tree/webpackconfig/dist |
Take a look at this comment in #22 |
yes that seems like it... 😒 I have been playing around a little and it seems we could just read the karma files and totally replace them with whatever webpack creates. Doing so should allow for a much easier interaction as we could provide the entry points simply via the webpack option instead of manually adding the entry and generating our own However, this strays pretty far from the original plugin now 🙈 So I am afraid if I continue that it's more like a rewrite then a bugfix - would you be at all will to accept such a huge change? Nonetheless here is the progress for today:
intersting parts // webpack plugin
class KarmaSyncPlugin {
apply(compiler) {
compiler.hooks.done.tap('KarmaSyncPlugin', stats => {
this.notifyKarmaAboutChanges();
});
}
// ...
}
const webpackOptions = {
mode: 'development',
plugins: [
new KarmaSyncPlugin({ karmaEmitter: /* here karma will pass it's emitter */ })
],
// ...
};
class KarmaWebpack {
constructor() {
const compiler = webpack(webpackOptions);
// ... ToDos:
|
I guess this is a question for the maintainer(s), really. As a user, I believe the benefits (speed, isolation) far outweigh the inconvenience of a breaking change, assuming there won't be unnecessary additional setup (e.g requiring the user to make fake entry files, when it should be done by Karma). In any case, @daKmoR i am more than happy to help test it out on our projects, which have vanilla web components and 1000s of tests between them. |
Awesome idea. I think the easiest method of approach is creating something
in node that takes the example karma configs you’ve been using to run into
this scenario, outputting a single file that’ll then run the multiple
outputted bundles. After that, we can modify `karma-webpack` to use that,
rather than trying to modify `karma-webpack` to do this.
I say this because the lack of API for karma and trying to understand the
existing `karma-webpack` codebase and the context behind each part has left
me wasting many hours before when trying to do stuff.
Please give me a shout if you need any help!
…On Wed, 5 Dec 2018 at 23:53, Theo ***@***.***> wrote:
I guess this a question for the maintainer(s), really.
As a user, I believe the benefits (speed, isolation) far outweigh the
inconvenience of a breaking change, assuming there won't be unnecessary
additional setup (e.g requiring the user to make fake entry files, when it
should be done by automatically by Karma).
In any case, @daKmoR <https://github.com/daKmoR> i am more than happy to
help test it out on our projects, which have vanilla web components and
1000s of tests between them.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#379 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AHjhvRpf0WzxbUIchvLrkcUsr6Ylkmp8ks5u2FyNgaJpZM4Y9tMu>
.
|
Sorry I wrote that previous comment via email and it doesn’t seem to have applied any formatting... |
@dogoku yes I think so too :) that's why I'm investing my evenings here :p (of course also to learn) and thanks for the encouraging words :) that motivates me even more :) so once something is ready I will definitely let u know. @rynclark I am still playing around with some hard coded files and sort of hacking it in the "node_modules" folder and so far it seem pretty doable (at least I am still optimistic) Once I have something to really review I will definitely let you know. You can probably follow me along while I am working on this pull request: https://github.com/daKmoR/karma-webpack-wc-bug/pull/1/files |
@daKmoR thanks for your work on this - if you need me at all DM me on twitter or something, not sure what everyone uses these days (apart from gitter please ha) |
@daKmoR I have already seen open-wc yesterday as I was checking your contribs (feels like a facebook stalker) and it's really impressive the amount of knowledge you have managed to collect! I actually wanted to ask about a couple of things (specifically around storybook integration) but I figured this isn't the thread to do so. |
@rynclark thx will let you know :) @dogoku thx :) we have an |
quick update I had to add to webpack
and load the file So getting it now to work for a "normal" karma config should be doable... I'm only a little worried for the interaction with other plugins especially for code coverage... as the code is now split into multiple files and I am not sure to how to make these extra files nicely available to everywhere 🙈 but let's see - I will probably try out the current existing workaround in karma-webpack to read the webpack generated file in a karma preprocessor... combined with the inline source maps that webpack generates it could work... let's see. |
Update of today:
Still missing:
|
Added features:
Overall it now fully supports all core features I would expect 🎉
@rynclark can you pls take a look at the 2 files KarmaWebpack Class and karma-webpack plugin If the solutions seems good I can prepeare a Pull Request to karma-webpack itself. PS: maybe we could then also get rid of the build step in karma-webpack? I think only supporting somewhat up to date node versions should be good enough. However then again that is a different topic. |
Wow, man you blew me away. This looks amazing work for such a short time. I'll test it out tomorrow and give you feedback. I'll try to run it on both mac & windows, and run both single-run and debug modes. @daKmoR if you need me to test anything in particular, let me know. |
@dogoku thank you for your kind words ❤️ this is exactly the reason why I love open source :) thank you :) I will prepare a Pull Request so it's also easier to test :) Nothing particular to tests... just be careful with custom configs... also you will need to make sure webpack is added as a framework and preprocessor. Still mind you I would still consider this as a POC as it's missing tests, linting, ... but it's a good starting point as we now know it's possible :) |
So does the I created a I'll try to fix the remaining bugs in |
We have to match |
changed
could we maybe release a 5.0.0-rc1 in a somewhat timely manner e.g. within a week or so? I would like to test it on a broader scale and a released version makes it way easier :)
I don't think there is anything used that needs a more up to date version? |
|
Does this need to be moved to |
(p.s. awesome work) |
Well some people started using the release candidates of 4.0.0 already, I don't think such a change should happen after a RC. |
also a possibility... so let's go without babel for now and think about that one afterwards
THXXX 💪 |
True @matthieu-foucault. What's left for |
I know that implies that |
|
…380) Closes #379 BREAKING CHANGE: webpack needs to be added to frameworks ``` // old: frameworks: ['mocha'], // new: frameworks: ['mocha', 'webpack'], ``` BREAKING CHANGE: old alternative usage is no longer recommended BREAKING CHANGE: webpack-dev-middleware removed BREAKING CHANGE: default webpack configuration changed drastically
Each test file get's bundled individually but placed in a single browser window.
This leads to errors when using a global registry or global variables/systems that need to be unique.
actual behavior
Error in test
Failed to execute 'define' on 'CustomElementRegistry': this name has already been used with this registry
expected behavior
Test run successfully even if some global variable is used
What happens
Failed to execute 'define' on 'CustomElementRegistry': this name has already been used with this registry
Steps to Reproduce
I made a minimum failing test:
https://github.com/daKmoR/karma-webpack-wc-bug
possible Solutions
These are mere ideas to spark a conversation:
1) manually create a single entry point
Besides the *.test.js files have also an index.js
Good
Bad
2) treat it as "Multi Page Application"
Use https://webpack.js.org/concepts/entry-points/#multi-page-application to create bundles that know of each other.
E.g. do not create bundles in isolation.
Good
Bad
System Info
The text was updated successfully, but these errors were encountered: