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 if passing a different webpack config filename in remove command #882

Closed
anikethsaha opened this issue May 10, 2019 · 5 comments
Closed

Comments

@anikethsaha
Copy link
Member

anikethsaha commented May 10, 2019

Describe the bug

webpack-cli remove webpack.prod.js    # any name other than webpack.config.js will throw error

What is the current behavior?
Throwing typeError and stopping the execution of the package.

To Reproduce
Steps to reproduce the behavior:

  1. Create a webpack config filename name other than webpack.config.js, something like webpack.prod.js or anything else
  2. run $ webpack-cli remove webpack.prod.js
    it will throw this.
√ SUCCESS Found config webpack.config.prod.js

internal/validators.js:125
    throw new ERR_INVALID_ARG_TYPE(name, 'string', value);
    ^

TypeError [ERR_INVALID_ARG_TYPE]: The "id" argument must be of type string. Received type object

Expected behavior
This is happeing because in remove-generator.ts if the config filename is not equal to webpack.config.js then it is assigning the configPath as null and then require(configPath) which is resulting in this error

let configPath = path.resolve(process.cwd(), "webpack.config.js");
		const webpackConfigExists = fs.existsSync(configPath);
		if (!webpackConfigExists) {
			configPath = null;
			// end the generator stating webpack config not found or to specify the config
		}
this.webpackOptions = require(configPath);   // typeError here 

Fix
One way to fix this can be.
to ask the user for the correct webpack config file name , something like this.

let configPath = path.resolve(process.cwd(), "webpack.config.js");
const webpackConfigExists = fs.existsSync(configPath);

if (!webpackConfigExists) {
  configPath = null;
  console.log("Couldn't find the webpack.config.js in your current directory ")
  this.webpackOptions = null
} else {
  this.webpackOptions = require(configPath);
}



prompting(){

  if (this.webpackOptions == null) {
    let configPath;
    this.prompt([Confirm("isOtherConfigFilePresent", "Do you have webpack config file with any other name ? ")])
      .then(answer => {
        const {
          isOtherConfigFilePresent
        } = answer;
        if (isOtherConfigFilePresent) {
          this.prompt([Input("configFileName", "Enter your config file name")])
            .then(answer2 => {
              const {
                configFileName
              } = answer2;
              configPath = configFileName;
            })
        }
      })
      this.webpackOptions = require(configPath);
  }
  

}

or

To simply thow a nice error description from if-statement

	if (!webpackConfigExists) {
			throw new Error("Please rename your config file with webpack.config.js");
		}

Not sure about these fix as I think yeoman generator will not work properly if we have this.prompt outsite of the prompting() method or may be in constructor

with Inquirer and using Inquirer.prompt() instead of yeoman 's prompt in contructor can also work

Please paste the results of webpack-cli info here, and mention other relevant information

  System:
    OS: Windows 10
    CPU: (4) x64 Intel(R) Core(TM) i5-6200U CPU @ 2.30GHz
  Binaries:
    Yarn: 1.15.2 - C:\Program Files (x86)\Yarn\bin\yarn.CMD
    npm: 6.9.0 - C:\Program Files\nodejs\npm.CMD
@anikethsaha anikethsaha changed the title typeError if passing a different webpack config filename in remove-generator typeError if passing a different webpack config filename in remove command May 10, 2019
@anikethsaha
Copy link
Member Author

cc @evenstensberg

@ematipico
Copy link
Contributor

ematipico commented May 12, 2019

PR is welcome. We should adopt the some solution we use for add/update.

We can't ask the user to rename their filename

@anikethsaha
Copy link
Member Author

Can we use inquirer js in the constructor? @ematipico
I tried with the yeomen ' s prompt but it wont workout. properly

@ematipico
Copy link
Contributor

ematipico commented May 12, 2019

I wouldn't prompt to the user. Let's just fix the file resolution. I think for now it's enough.

@anikethsaha
Copy link
Member Author

okk

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

No branches or pull requests

2 participants