Only include dev-server parts if for dev server

[justin808]

Without this change, webpack still includes things like window
in the build even if not using the webpack-dev-server.

When that happens, the bundle cannot be used for server-side rendering.

[justin808]

See #2629 for why this variable is there.

and

webpack/webpack-dev-server#1929

[justin808]

@gauravtiwari This allows a simple setup to work for SSR. Otherwise, SSR can work if HMR and inline are set to false for the devServer. That's how I figured out this was needed.

[justin808]

@gauravtiwari @rossta @JakeLaCombe any chance of a quick review on this? It blocks me from making a super cool demo of React on Rails v12 where server rendering just works, so long as the webpack dev server is not used. It's a little more complicated with that one!

[justin808]

How do I fix the ruby specs?

[justin808]

When I ran the tests, for some reason, the undefined comes as a string for comparison.

[justin808]

This is per the definition here: https://github.com/webpack/webpack-dev-server/pull/1929/files#diff-15fb51940da53816af13330d8ce69b4eR66

if (!process.env.WEBPACK_DEV_SERVER) {
  process.env.WEBPACK_DEV_SERVER = true;
}

[justin808]

@gauravtiwari @jakeNiemiec Any feedback on why this is not getting merged?

[gauravtiwari]

Thanks for this @justin808 and apologies for delay.

The code looks good, the only reason I have been hesitating is the dependence on env variable. I was thinking perhaps, we should have a completely different config class for dev server which extends development.js and then in the dev server config we instantiate that class explicitly.

https://github.com/rails/webpacker/blob/master/lib/webpacker/runner.rb#L14

      @webpack_config        = File.join(@app_path, "config/webpack/#{ENV["NODE_ENV"]}.js")

This line here should be overridden in dev server runner and set to dev server config class regardless of NODE_ENV.

@webpack_config        = File.join(@app_path, "config/webpack/dev_server.js")

[justin808]

@gauravtiwari I think this change is small enough that we could merge this PR and do a bigger change as you suggest next. AFAIK, I don't think the webpack-dev-server would remove that ENV value as many other apps might depend on it.

I could try to work on your suggestion as a followup PR.

For right now, it's very confusing that the HMR settings break the non-HMR non-webpack-dev-server usage of bin/webpack.

[justin808]

Thanks @gauravtiwari!

[aried3r]

This change broke our test suite, since WEBPACK_DEV_SERVER isn't set during tests anymore after this.

What exactly is the migration path from webpacker 5.1.1 to 5.2.1 regarding this and where to best set this variable in a "webpacker-way"?

The code causing our issues is the following in config/webpack/development.js:

const chokidar = require("chokidar")
environment.config.devServer.before = (app, server) => {
  chokidar
    .watch([
      "config/locales/**/*.yml",
      "app/views/**/*.html.erb",
      "app/assets/**/*.scss",
    ])
    .on("change", () => server.sockWrite(server.sockets, "content-changed"))
}

Error:

TypeError: Cannot set property 'before' of undefined
    at Object.<anonymous> (config/webpack/development.js:10:37)

Taken from #1879 (comment)

I don't know how development.js is influencing the test env in this case. The following fixes the issue.

$ WEBPACK_DEV_SERVER=true rspec

[aried3r]

Okay, in classic fashion of rubber duck debugging, I found the solution shortly after writing it down.

webpacker/lib/install/config/webpack/test.js

Line 1 in 9ec5daa

process.env.NODE_ENV = process.env.NODE_ENV || 'development'

is causing it and we'll write a guard clause similar to what's in this PR for our use case.

[justin808]

@aried3r how does setting the before on the dev_server object work?

[justin808]

@aried3r the NODE_ENV of test picks the test.js file by default. Resetting the NODE_ENV to 'development' in the test.js file seems very odd.

[aried3r]

Hey @justin808, thank you for your comment. I believe #2721 and #2654 answer my question much better than I did in my follow-up comment.

how does setting the before on the dev_server object work?

I don't quite understand, could you elaborate?

[justin808]

@aried3r, what code uses the function set in property environment.config.devServer.before?