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

TypeError: Class constructor ConfigList cannot be invoked without 'new' #1097

Closed
rossta opened this issue Dec 13, 2017 · 13 comments
Closed

TypeError: Class constructor ConfigList cannot be invoked without 'new' #1097

rossta opened this issue Dec 13, 2017 · 13 comments
Labels

Comments

@rossta
Copy link
Member

rossta commented Dec 13, 2017

System:

@rails/webpacker version: 3.1.1
• node version: v6.11.5
• lodash version: 4.14.2

Steps to reproduce:

const { cloneDeep } = require('lodash');
const { environment } = require('@rails/webpacker');

cloneDeep(environment.toWebpackConfig(), {});

Sample output:

/dev/node_modules/lodash/lodash.js:5763
          result = array.constructor(length);
                         ^

TypeError: Class constructor ConfigList cannot be invoked without 'new'
    at initCloneArray (/dev/node_modules/lodash/lodash.js:5763:26)
    at baseClone (/dev/node_modules/lodash/lodash.js:2396:18)
    at /dev/node_modules/lodash/lodash.js:2439:34
    at arrayEach (/dev/node_modules/lodash/lodash.js:485:11)
    at baseClone (/dev/node_modules/lodash/lodash.js:2433:7)
    at /dev/node_modules/lodash/lodash.js:2439:34
    at arrayEach (/dev/node_modules/lodash/lodash.js:485:11)
    at baseClone (/dev/node_modules/lodash/lodash.js:2433:7)
    at cloneDeep (/dev/node_modules/lodash/lodash.js:10650:14)
    at Object.<anonymous> (/dev/tmp/webpacker_clonedeep.js:4:18)

Attempting a deep clone of the object returned by environment.toWebpackConfig() results in a TypeError. I came across this error by using webpack-merge, which delegates to lodash cloneDeep, to update the Webpack config.

Perhaps it makes sense to ensure that once toWebpackConfig is called, instances of Webpacker base type subclasses, e.g., ConfigList, ConfigObject, are converted back to their base types to avoid surprising results.

@guilleiguaran
Copy link
Member

Thanks for opening this!!

If you have an idea about how to fix this we would love to review a PR ❤️

@rossta
Copy link
Member Author

rossta commented Dec 13, 2017

@guilleiguaran Looking into it

@gauravtiwari
Copy link
Member

We just need to return native types back, didn't realise it's going to be used outside of webpacker though :)

@gauravtiwari
Copy link
Member

@rossta Would be interesting to know your use case with clonedeep

@rossta
Copy link
Member Author

rossta commented Dec 13, 2017

@gauravtiwari We've been using webpack-merge to merge in our extensions to Webpack config, which delegates to clonedeep.

@gauravtiwari
Copy link
Member

You could use built-in methods for merging custom configs: https://github.com/rails/webpacker/blob/master/docs/webpack.md#configuration

Is this enough?

@gauravtiwari
Copy link
Member

// config/webpack/custom.js
module.exports = {
  resolve: {
    alias: {
      jquery: 'jquery/src/jquery',
      vue: 'vue/dist/vue.js',
      React: 'react',
      ReactDOM: 'react-dom',
      vue_resource: 'vue-resource/dist/vue-resource',
    }
  }
}

// config/webpack/environment.js
const environment = require('./environment')
const customConfig = require('./custom')

// Set nested object prop using path notation
environment.config.set('resolve.extensions', ['.foo', '.bar'])
environment.config.set('output.filename', '[name].js')

// Merge custom config
environment.config.merge(customConfig)

@guilleiguaran
Copy link
Member

guilleiguaran commented Dec 13, 2017

+1 in support for webpack-merge since most of the examples on internet showing how to extend webpack configs use it.

It might seem a bit surprising that webpack-merge don't work with webpacker.

@gauravtiwari
Copy link
Member

Yep, made a PR but we ship a new method merge that can help remove webpack-merge as dependency.

@rossta
Copy link
Member Author

rossta commented Dec 13, 2017

Whether or not webpack-merge is officially supported isn't so much the concern as avoiding surprising results. I'd expect the object returned by toWebpackConfig to behave the same way as if I'd built it up by hand.

@renchap
Copy link
Contributor

renchap commented Dec 14, 2017

Another use-case is for webpacker-react. We currently use webpack-merge to update the configuration to use hot module reloading: https://github.com/renchap/webpacker-react/blob/master/javascript/webpacker_react-npm-module/src/configure-hot-module-replacement.js

const webpackConfigForHMR = require('webpacker-react/configure-hot-module-replacement.js')
const config = environment.toWebpackConfig()

module.exports = webpackConfigForHMR(config)

I guess we should switch to something else that uses Webpacker's the new API. Something like:

const patchWebpackerConfigForHMR = require('webpacker-react/patch-webpacker-config-for-hmr')

patchWebpackerConfigForHMR(environment)
module.exports = environment.toWebpackConfig()

@rossta
Copy link
Member Author

rossta commented Dec 18, 2017

@renchap This should be fixed on master

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants