TheLarkInn / webpack-workshop-2018

Learning resources for the webpack academy workshop series for 2018
MIT License
554 stars 995 forks source link

module.exports default params issue #28

Open timurcatakli opened 6 years ago

timurcatakli commented 6 years ago

@TheLarkInn thanks a lot for the course. It was very useful and your teaching style is amazing. Congratulations. One question, for some reason in my webpack config file below code didn't work. presets returned undefined

module.exports = ({ mode, presets } = { mode: "production", presets: [] }) => { but this one did: module.exports = ({ mode = 'production', presets = [] }) => { Any ideas?

Thanks

gabberr commented 5 years ago

First case does not work as expected. If the passed in env has undefined value for 'mode' or 'presets', the 'pulled out' values will stay undefined.

Correct version would be: module.exports = ({ mode: mode = "production", presets: presets = []} = { mode, presets})

I like the second, less hipster version more.

Example is here: https://repl.it/@gabberr/webpack-env-destructuring

@TheLarkInn I can open PR for this, which version should go in?

Christopher-Shea commented 5 years ago

Wish I had come to this thread earlier! At first I thought it might just be a node thing, but the following code does not work as expected in the browser either:

let env = {mode: 'mode'}
testConfig = ({mode, presets} = {mode: 'production', presets: []}) => {
  console.log(mode);
  console.log(presets)
}

testConfig(env) // 'mode' undefined
testConfig() // 'production' []

If no env variable is passed in, the mode and presets properties are correctly assigned the default values.

However, if any sort of env is passed in, the default values will not be applied to undefined properties of that env variable. This is because the default is only applied when the expression to the left of the assignment is wholly undefined. As long as some sort of env is passed, the default assignment will not be made to the destructured properties.

The provided alternative

module.exports = ({ mode = 'production', presets = [] }) =>

properly sets the default presets property if one isn't passed in, but cannot handle a runtime where no env is passed in - will throw a destructuring err (cannot read properties mode / presets of undefined or null)

For myself, I decided the most straightforward way to handle this was to keep the provided destructuring syntax

({mode, presets} = {mode: 'production', presets: []})

so that the config can handle absent env variables. To adjust for the undefined presets, I added the following line before the call to webpackMerge:

presets = presets || [];

There is an identical line in the loadPresets.js file that can be omitted once this is included.