Uh oh!
There was an error while loading. Please reload this page.
fix(client): stricter reg exp to redirect sockjs client path - #2069
Conversation
Codecov Report
@@ Coverage Diff @@## master #2069 +/- ##
==========================================
+ Coverage 94.56% 94.65% +0.08%
==========================================
Files 32 32 Lines 1215 1215 Branches 340 340 ==========================================
+ Hits 1149 1150 +1 + Misses 64 63 -1
Partials 2 2
Continue to review full report at Codecov.
|
| ); | ||
| }), | ||
| new webpack.NormalModuleReplacementPlugin( | ||
| /^\.\/clients\/SockJSClient$/, |
There was a problem hiding this comment.
We need search way how we can check what module was loaded from our client-src - it is right way don't break other code
There was a problem hiding this comment.
You mean to test that it is loading the correct module in the webpack compilation? Could we run this webpack config in a test, look at the webpack CLI output, and see if it contains the right path?
There was a problem hiding this comment.
Yes, but it can solved using NormalModuleReplacementPlugin as you use, just need more checks (need check context contains 'webpack-dev-server/client-src' and rewrite only in this case)
There was a problem hiding this comment.
@evilebottnawi I did something like this, except instead I matched '/client-src/default', because it is possible the dev server is not in a webpack-dev-server directory if someone clones it into a different named directory. If we want to be very strict about it, we could match against path.resolve to get the exact context we want, but I'm afraid that could result in some problem.
Also, I included backwards slash (\\) in my matching string because windows seems to use that for the context paths
67c9759 to
9721b30Compare9721b30 to
ecf61e3Compare
alexander-akait
left a comment
There was a problem hiding this comment.
Just check if (resource.context.startsWith(process.cwd())) { }, we need rewrite clients only for webpack-dev-server, for other package it should be not rewritting
knagaitsev
commented
Jul 9, 2019
Done. Good solution! |
…#2069) * fix(client): stricter reg exp to redirect sockjs client path * fix(client): check resource context for normal module replacement * fix(client): remove dev server string from context path * fix(client): remove console logs * fix(client): fixed resource context match to use cwd
For Bugs and Features; did you add new tests?
Yes
Motivation / Use-Case
Fixes problem introduced by #2015 using a stricter reg exp.
I added an iframe e2e test because the issue came up from users using
iframemode: #2006Breaking Changes
None
Additional Info