Skip to content

fix(client-src): don't use self.location.port#1838

Merged
hiroppy merged 1 commit into
masterfrom
revert/urlParts.port
May 7, 2019
Merged

fix(client-src): don't use self.location.port#1838
hiroppy merged 1 commit into
masterfrom
revert/urlParts.port

Conversation

@hiroppy

@hiroppy hiroppy commented May 1, 2019

Copy link
Copy Markdown
Contributor

ISSUE: #1777
REF: #1792
REF: #1664

  • This is a bugfix
  • This is a feature
  • This is a code refactor
  • This is a test update
  • This is a docs update
  • This is a metadata update

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

@hiroppy

hiroppy commented May 1, 2019

Copy link
Copy Markdown
Contributor Author

I checked using @njugray's repository and confirmed that this problem has been fixed.

@hiroppy hiroppy marked this pull request as ready for review May 1, 2019 17:02
@hiroppy hiroppy requested a review from alexander-akait as a code owner May 1, 2019 17:02

@alexander-akait alexander-akait left a comment

Copy link
Copy Markdown
Member

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

@hiroppy

hiroppy commented May 1, 2019

Copy link
Copy Markdown
Contributor Author

hmmm, ci is red. but since tests are passing at my local, I will investigate this issue...

@alexander-akait

Copy link
Copy Markdown
Member

/cc @hiroppy please ping me when it was done, thanks for working!

@hiroppy

hiroppy commented May 2, 2019

Copy link
Copy Markdown
Contributor Author

Yep, it is a difficult problem(because it just occurs on CI, and a port problem) so maybe I will use enough time.

@alexander-akait

Copy link
Copy Markdown
Member

@hiroppy friendly ping

@hiroppy hiroppy force-pushed the revert/urlParts.port branch 3 times, most recently from d979ed9 to 00aeb7b Compare May 7, 2019 15:20
Comment thread test/Client.test.js
@@ -62,7 +71,7 @@ describe('Client code', () => {
.waitForRequest((requestObj) => requestObj.url().match(/sockjs-node/))

@hiroppy hiroppy May 7, 2019

Copy link
Copy Markdown
Contributor Author

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.

@hiroppy hiroppy force-pushed the revert/urlParts.port branch from 00aeb7b to 9bd51e7 Compare May 7, 2019 15:50
@codecov

codecov Bot commented May 7, 2019

Copy link
Copy Markdown

Codecov Report

Merging #1838 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           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.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 34058a6...9bd51e7. Read the comment docs.

@hiroppy hiroppy requested a review from alexander-akait May 7, 2019 15:51
@hiroppy

hiroppy commented May 7, 2019

Copy link
Copy Markdown
Contributor Author

@evilebottnawi PTAL

@alexander-akait

Copy link
Copy Markdown
Member

Good job, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

V3.3.0 live reload broken when set hostname to 0.0.0.0:xxxx

2 participants