jsf-clabot
commented
6 years ago
All committers have signed the CLA.
Closed pat841 closed 6 years ago
jsf-clabot
commented
6 years ago
All committers have signed the CLA.
JayCanuck
commented
6 years ago Just wanted to add my 2cents here. Since #279 karma-webpack has been broken for myself and colleagues on Windows (Windows was resolving C:/.... into file://C:/ urls and failing for XHR). Was going to create a new issue but saw this PR. Looks like this patch fixes that issue flawlessly too. All tests resume passing with this PR branch. Edit: looks like there's a related issue of others seeing the same Windows ajax issues that this PR solves https://github.com/webpack-contrib/karma-webpack/issues/317
lsphillips
commented
6 years ago I was about to submit a PR, good thing I checked first :-).
Would really like to see this merged as soon as possible as this makes this project unusable for Windows users, especially as code splitting is a common practice these days.
michael-ciniawsky
commented
6 years ago cc @rynclark
ryanclark
commented
6 years ago LGTM after conflicts are resolved
pat841
commented
6 years ago @rynclark Should be all set.
JayCanuck
commented
6 years ago @rynclark Tests are failing for me with the latest commit. Looks to be because path.join is used on webpackMiddlewareOptions.publicPath, which on Windows results in backslashes (\) and that backslash-based path fails to match the forward-slash (/) requests. Can you modify that line to be a static '/_karma_webpack_/' like it was before the rebase?
pat841
commented
6 years ago @JayCanuck Yeah I was unsure about using path.join() for that reason but wanted to stay within the re-base updates. @rynclark Can I remove the path.join() calls around publicPath setters?
JayCanuck
commented
6 years ago At least in local testing, it looks like the webpackOptions.output.publicPath = path.join(... is fine and webpack handles backslashes correctly, but for handing request public path (and all the things that it extends to), the path.join(...) on webpackMiddlewareOptions.publicPath seems to be the key problem on Windows and needs forward slashes.
pat841
commented
6 years ago Theres also the option of .replace(/\\/g, '/');
JayCanuck
commented
6 years ago That's true, yea, though that feels like having the unnecessary path.join and replace operations for unneeded processing compared to an equivalent static string.
michael-ciniawsky
commented
6 years ago @pat841 Please leave it as is within this PR. It is a bug introduced by another recent PR and this ideally should to be adressed separately
pat841
commented
6 years ago @michael-ciniawsky Would you like me to create a new PR for the Windows fix based off of this branch?
michael-ciniawsky
commented
6 years ago Released in 4.0.0-rc.2 🎉
johnnyreilly
commented
6 years ago I've just tested 4.0.0-rc.2 on Windows; still a problem I'm afraid. It's what we use in ts-loader for our execution test pack.
You can see this PR upgrades from 2.0.6 to 4.0.0-rc2 and then experiences issues:
https://github.com/TypeStrong/ts-loader/pull/840
Linux is fine; Windows is not. The failing tests all relate to code splitting / dynamic imports.
I'll open a new issue to track this and link back.
Please check if the PR fulfills these requirements
What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
What is the current behavior? (You can also link to an open issue here)
This fixes issue #333 where a worker would fail to load due to a CORS issue. This change does NOT affect the write path for the webpack build, but the public path reflected in something like
<base href="/_karma_webpack/">.What is the new behavior?
The
os.tmpdir()is stripped from the webpack's publicPath so that async assets are loaded correctly from a common domain avoiding CORS issues.Does this PR introduce a breaking change?
If this PR contains a breaking change, please describe the following...
Other information: