fix: allow single object proxy config

[lukebro]

• 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?

Yes.

• I added a new test to test the proxy config.
• I renamed a previous test since the name of the test was too generic after adding a new one.

Motivation / Use-Case

Fix #1438. Allow for a simple object proxy config as described in the options schema and demonstrated in the docs.

module.exports = {
  //...
  devServer: {
    index: '', // specify to enable root proxying
    host: '...',
    contentBase: '...',
    proxy: {
      context: () => true,
      target: 'http://localhost:1234'
    }
  }
};

Breaking Changes

N/A

Additional Info

I'm not sure if just checking for a target prop is too naive on the proxy config.

[alexander-akait]

Thanks for PR, good job, we need fix CI problem before merge

[lukebro]

@evilebottnawi I'm not sure what the issue is with CI, it timed out.

I ran tests on both Node.js 10.15.0 and everything is green.

It doesn't make much sense because the LTS one passed and it's also 10.15.0.

Can you retrigger the CI?

[alexander-akait]

@lukebro done, somethings wrong with Travis, let's wait rebuild, feel free to ping me after build

[codecov]

Codecov Report

Merging #1633 into master will increase coverage by 0.08%.
The diff coverage is 90%.

@@            Coverage Diff             @@
##           master    #1633      +/-   ##
==========================================
+ Coverage   75.37%   75.45%   +0.08%     
==========================================
  Files          19       19              
  Lines         597      599       +2     
  Branches      172      173       +1     
==========================================
+ Hits          450      452       +2     
  Misses        113      113              
  Partials       34       34

Impacted Files	Coverage Δ
lib/Server.js	78.55% <90%> (+0.11%)	⬆️

---

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 017dc3d...1ea6f06. Read the comment docs.

[lukebro]

@evilebottnawi failed again by timeout. How do you recommend I debug this?

[alexander-akait]

@lukebro do you have mac os machine? We need run this on this system and look what is happening

[lukebro]

@evilebottnawi Yeah I'm on macOS 10.14.2 with Node.js v10.6.0. I've tried with Node.js v10.15.0 also and I can't reproduce the timeout.

I'm going to setup my own Travis CI build to debug, I'll ping you when I find something.

[alexander-akait]

@lukebro thanks for helping

[alexander-akait]

@lukebro looks something wrong with proxy middleware on mac os, because last build is green #1640

[lukebro]

Yeah @evilebottnawi it is the proxy middleware.

When the test helpers call close the middleware just hangs and callback is never called:
https://github.com/lukebro/webpack-dev-server/blob/841c585ba0a2754f91b9be99f8326221cea779c5/lib/Server.js#L829-L831

Should I open an issue there and look into it there?

If I remove my changes and create a test with this config:

module.exports = {
  //...
  devServer: {
    index: '', // specify to enable root proxying
    host: '...',
    contentBase: '...',
    proxy: [{
      context: () => true,
      target: 'http://localhost:1234'
    }]
  }
};

This should have the same effect (from proxy middleware perspective) and fail.

[alexander-akait]

@lukebro hm, don't know without this fix all works fine, maybe something wrong with tests, need investigate how we can reproduce bug without dev server

[lukebro]

The good news is I'm able to now consistently reproduce the timeout locally on my machine, still debugging.

Nevermind. I was able to track the timeout all the way to NodeWatchFileSystem.js before it just started going green locally again. There could be something wrong with the tests causing some type of race condition (hence inconsistency in which macOS job fails) with the watcher. Not really sure.

For example:
macOS Node 10 fails: https://travis-ci.org/webpack/webpack-dev-server/jobs/484302838
macOS Node 10 passes: https://travis-ci.org/lukebro/webpack-dev-server/jobs/484958421

Furthermore, I kept the changes I made but removed the the test change and the tests on CI are all green.
https://travis-ci.org/lukebro/webpack-dev-server/builds/484961124

@evilebottnawi would you be okay with me debugging and possibly refactoring the tests in this PR to address the issues? I can also take a look at the ConcurrentCompilationError errors that the current set of tests are throwing.

[alexander-akait]

@lukebro

I can also take a look at the ConcurrentCompilationError errors that the current set of tests are throwing.

For some tests it is normal.

Okey, let's comment tests and add todo with link on this issue and i think we can merge

[lukebro]

I'm going to get back to this tomorrow.

[alexander-akait]

Need rebase too 👍

[lukebro]

@evilebottnawi sorry, better late then never! Apex came out and I got really distracted.

Since the jump to Jest it looks like the failing tests on 10, 8, 6 are now passing 😊. The failure for 11 is extremely weird, and might not be related. Can you quickly take a look and possibly rebuild?

[alexander-akait]

@lukebro Looks something wrong with node on macos 😕 Rebuild doesn't solve problem, maybe related #1660, can you try it locally?

[hiroppy]

Node11 is often unstable only on CI 🤔

[alexander-akait]

@hiroppy other on macos, and it is very strange, maybe we can investigate which test break CI?

[alexander-akait]

Somebody can test this locally?

[hiroppy]

@evilebottnawi 8 and 10 are good, but 11 is not good.
Ok, I'll investigate flaky tests.

[lukebro]

@hiroppy let me know if I can help investigate in any way.

@evilebottnawi For me 8, 10 and 11 all pass locally. Like Yuta mentioned this seems to be a CI problem with 11.

[hiroppy]

fyi: nodejs/node#24835

[hiroppy]

@evilebottnawi

The results of the investigation are as follows.

Proxy.test.js has libuv errors.

Assertion failed: (handle->flags & UV_HANDLE_CLOSING), function uv__finish_close, file ../deps/uv/src/unix/core.c, line 253.

HistoryApiFallback.test.js has segmentation fault.

[1]    40029 segmentation fault  npm test

[alexander-akait]

@hiroppy Great work! Maybe we can skip test what break CI? Like if (node === 11 && isMacOS) { return expect(true).toBe(true); }

[lukebro]

Waiting for #1667 to be merged.

[alexander-akait]

/cc @lukebro please rebase

[lukebro]

@evilebottnawi all set 👍