Skip to content
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

webpack Multiple Targets #1256

Closed
sanonz opened this issue Jan 5, 2018 · 9 comments
Closed

webpack Multiple Targets #1256

sanonz opened this issue Jan 5, 2018 · 9 comments

Comments

@sanonz
Copy link

sanonz commented Jan 5, 2018

Code

// webpack.config.js
module.exports = [
  {
    entry : {
      app: './src/app'
    },
    output: {
      path: path.resolve(__dirname, 'dist/')
      filename: 'assets/js/app.js',
      chunkFilename: `assets/js/${getAssetName('[hash]', '[name]')}.js`
    },
    devServer: {
      hot: true
    }
  },
  {
    entry : {
      app: './src/app.server'
    },
    output: {
      path: path.resolve(__dirname, 'dist/server')
      filename: 'app.js',
      chunkFilename: '[name].js',
      libraryTarget: 'commonjs2'
    },
    target: 'node'
  }
];

Run

npx webpack-dev-server

Expected Behavior

Actual Behavior

All added.

For Bugs; How can we reproduce the behavior?

Example edit: https://github.com/webpack/webpack-dev-server/blob/master/lib/util/addDevServerEntrypoints.js#L22

    [].concat(webpackOptions).forEach((wpOpt) => {
+     if (wpOpt.target === 'node') return;
      if (typeof wpOpt.entry === 'object' && !Array.isArray(wpOpt.entry)) {
        Object.keys(wpOpt.entry).forEach((key) => {
          wpOpt.entry[key] = devClient.concat(wpOpt.entry[key]);
        });
      } else if (typeof wpOpt.entry === 'function') {
        wpOpt.entry = wpOpt.entry(devClient);
      } else {
        wpOpt.entry = devClient.concat(wpOpt.entry);
      }
    });

For Features; What is the motivation and/or use-case for the feature?

server side rendering

@sanonz sanonz closed this as completed Jan 5, 2018
@sanonz
Copy link
Author

sanonz commented Feb 28, 2018

Turned out to be such, no add:

// webpack.config.js

{
+    entry: (devClient) => {
+      return {
+        app:  './src/setup.server',
+      };
+    },
    output: {
      path: path.resolve(__dirname, 'dist/server')
      filename: 'app.js',
      chunkFilename: '[name].js',
      libraryTarget: 'commonjs2'
    },
    target: 'node'
  }

The update cannot be used:
f4f14ce#diff-6a9b3dfc5b6956a454f4233ea7a4992cR24

@sanonz sanonz reopened this Feb 28, 2018
@sanonz sanonz changed the title webpack multiple config webpack Multiple Targets Feb 28, 2018
@SpaceK33z
Copy link
Member

So for the Node target it shouldn't auto-add the webpack/hot scripts? Seems fair enough I think. However this could potentially be a breaking change to some. Are there any use cases where you would need it to work like before?

@sanonz
Copy link
Author

sanonz commented Feb 28, 2018

I want to creating isomorphic apps, which contains two webpack configuration files, one for client app and one for server-side app. The server side app runs in a node environment, so the webpack/hot scripts is not required.

@SpaceK33z
Copy link
Member

Yes, but I was wondering if there are valid use cases where you do need these webpack/hot scripts in a node environment. If there are none, this isn't really a breaking change. If there are, it might be a breaking change.

@sanonz
Copy link
Author

sanonz commented Feb 28, 2018

The Node target add https://github.com/webpack/webpack-dev-server/blob/master/client-src/default/index.js file.
The runs in node environment throw error self not defined at line: https://github.com/webpack/webpack-dev-server/blob/master/client-src/default/index.js#L180
Node environment need client code?

@SpaceK33z
Copy link
Member

Okay thanks for finding out. Could you make a PR and add a test for this?

@sanonz
Copy link
Author

sanonz commented Feb 28, 2018

Sorry, I question write wrong. I make PR and test at tomorrow. Thanks!

@sethomas
Copy link

+1 I am also experiencing the same thing.

@SpaceK33z what's the status on merging @sanonz PR?

@alexander-akait
Copy link
Member

Fixed in #1775

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