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

Pass webpack into webpack() fn in next.config.js #456

Closed
wants to merge 1 commit into from

Conversation

ericf
Copy link
Contributor

@ericf ericf commented Dec 21, 2016

This passes a refernce to the webpack module into a custom webpack() function defined in next.config.js.

A user wanting to add something like a custom DefinePlugin entry might have a next.config.js file that looks like this:

module.exports = {
  webpack: (cfg) => {
    cfg.plugins.push(
      new webpack.DefinePlugin({
        'process.env.CUSTOM_VALUE': JSON.stringify(process.env.CUSTOM_VALUE),
      })
    );

    return cfg;
  },
};

The problem is they need access to the webpack module. They might think to require('webpack') in their next.config.js, but then they might feel obligated to add webpack to their package.json. Instead, it might be simpler to just pass a reference to the webpack to their config like this:

module.exports = {
  webpack: (cfg, {webpack}) => {
    // ...
  },
};

Curious what folks think about this. Maybe someone is worried that the user would try to call the webpack() module ref and kick off the build?

This passes a refernce to the `webpack` module into a custom
`webpack()` function defined in `next.config.js`.

A user wanting to add something like a custom `DefinePlugin` entry
might have a `next.config.js` file that looks like this:

```js
module.exports = {
  webpack: (cfg) => {
    cfg.plugins.push(
      new webpack.DefinePlugin({
        'process.env.CUSTOM_VALUE': JSON.stringify(process.env.CUSTOM_VALUE),
      })
    );

    return cfg;
  },
};
```

The problem is they need access to the `webpack` module. They might
think to `require('webpack')` in their `next.config.js`, but then
they might feel obligated to add `webpack` to their `package.json`.
Instead, it might be simpler to just pass a reference to the
`webpack` to their config like this:

```js
module.exports = {
  webpack: (cfg, {webpack}) => {
    // ...
  },
};
```
@coveralls
Copy link

Coverage Status

Coverage increased (+2.5%) to 60.668% when pulling c353c3a on ericf:config-webpack-ref into 95eb20e on zeit:master.

@@ -397,8 +397,13 @@ The following example shows how you can use [`react-svg-loader`](https://github.

```js
module.exports = {
webpack: (cfg, { dev }) => {
webpack: (cfg, { webpack, dev }) => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this core idea here. It makes totally sense.
But I'm not quite sure about the name of it.

May be it could be webpackNpmModule.

@nkzawa
Copy link
Contributor

nkzawa commented Dec 21, 2016

What if it's import webpack from 'next/webpack' ?

@ericf
Copy link
Contributor Author

ericf commented Dec 21, 2016

@nkzawa sure! Side question: does next.config.js get compiled? If not then require('next/webpack') 😄

@rauchg
Copy link
Member

rauchg commented Dec 21, 2016

+1 for using import system. Should we just embrace "npm3 flat structure side effects" and go for import webpack from 'webpack' ?

@nkzawa
Copy link
Contributor

nkzawa commented Dec 21, 2016

@ericf yeah, it has to be require()

Should we just embrace "npm3 flat structure side effects" and go for import webpack from 'webpack' ?

@rauchg I think this is not a good idea since it requires not to have webpack in dependencies of other modules or application.

@ericf
Copy link
Contributor Author

ericf commented Dec 21, 2016

Should we just embrace "npm3 flat structure side effects"

@rauchg that's what I'm doing right now. My main concern was someone wanting to add webpack to their package.json and then potentially ending up being out of sync with the version Next uses (especially with a yarn.lock file.)

Also, we can only use import in Node if we plan to first pass the config through a compile step. I can certainly update the PR to process the config file with Babel if that's something you guys want to do. If we embrace import we should probably embrace export as well, and update the docs to use:

export default {};

…instead of:

module.exports = {};

This is similar to how rollup.config.js works, it supports import/export.

@nkzawa
Copy link
Contributor

nkzawa commented Dec 21, 2016

@ericf we don't transpile the config file by design for now because we're going to have custom babel setting on the file but can't use the setting to transpile the file itself.

@ericf
Copy link
Contributor Author

ericf commented Dec 21, 2016

@nkzawa yeah that would be a gap. I see three main ways forward to resolve this:

  1. Keep things as is, people can use require('webpack') in next.config.js and rely on npm 3 hoisting.

  2. Add require('next/webpack') module that's an alias for require('webpack'), keeps people from the urge of adding webpack to their package.json.

  3. Pass-in the reference to the webpack module to the webpack() function in next.config.js.

@rauchg
Copy link
Member

rauchg commented Dec 21, 2016

@nkzawa:

@ericf we don't transpile the config file by design for now because we're going to have custom babel setting on the file but can't use the setting to transpile the file itself.

@rauchg
Copy link
Member

rauchg commented Dec 22, 2016

@ericf I think the best solution is to expose it as next/webpack. It's better than using it as a parameter methinks

@rauchg rauchg closed this Dec 22, 2016
@nkzawa
Copy link
Contributor

nkzawa commented Dec 22, 2016

Btw, I think we have a naming issue since we have next/babel. It should be next/webpack-module or next/babel-preset ?

@ericf ericf deleted the config-webpack-ref branch December 22, 2016 06:20
@radum
Copy link

radum commented Jun 18, 2017

I don't think this has been done has it? Exposing webpack to next/webpack.

As many others said this will be very helpful for new webpack.DefinePlugin({

@lock lock bot locked as resolved and limited conversation to collaborators May 12, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants