-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
fix(client-src): don't use self.location.port #1838
Conversation
I checked using @njugray's repository and confirmed that this problem has been fixed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, let's wait CI green
hmmm, ci is red. but since tests are passing at my local, I will investigate this issue... |
/cc @hiroppy please ping me when it was done, thanks for working! |
Yep, it is a difficult problem(because it just occurs on CI, and a port problem) so maybe I will use enough time. |
@hiroppy friendly ping |
d979ed9
to
00aeb7b
Compare
@@ -62,7 +71,7 @@ describe('Client code', () => { | |||
.waitForRequest((requestObj) => requestObj.url().match(/sockjs-node/)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When the port is 9000
, requestObj.url()
is http://localhost:9000/main.js
(so, match
returns null
) and then requestObj.url()
will be http://localhost:9001/sockjs-node/info?t=xxxx
after this page will be proxyed.
00aeb7b
to
9bd51e7
Compare
Codecov Report
@@ Coverage Diff @@
## master #1838 +/- ##
=======================================
Coverage 89.31% 89.31%
=======================================
Files 11 11
Lines 627 627
Branches 187 187
=======================================
Hits 560 560
Misses 55 55
Partials 12 12 Continue to review full report at Codecov.
|
@evilebottnawi PTAL |
Good job, thanks! |
ISSUE: #1777
REF: #1792
REF: #1664
For Bugs and Features; did you add new tests?
Revert from #1664.
Motivation / Use-Case
this pr is just a revert, but it is WIP status.(to validate)
Breaking Changes
nope
Additional Info
fixes #1777