fix(generator): using configFile in configPath to get the config file name #883

anikethsaha · 2019-05-12T11:40:09Z

Passing {configFile:configFile} to the env.run(generator) as second
argument in packages/utils/modify-config-helper.ts and using this property in generator class to get the
configPath in generator class constructor in remove-generator.ts

What kind of change does this PR introduce?
bugfix

Did you add tests for your changes?
No

If relevant, did you update the documentation?
Not required

Summary
Issue link : #882
In order to avoid the typeError error while running the remove command when passing the config file name other than webpack.config.js
Added a configFile property while running the generator from modify-config-helper.ts :

	env.registerStub(generator, generatorName);

	env.run(generatorName,{
		configFile
	}).then()

In order to pass the user's config file name to the generator class and replace the hardcoded webpack.config.js to this

	const {configFile} = opts
		let configPath = path.resolve(process.cwd(), configFile);
		const webpackConfigExists = fs.existsSync(configPath);

Does this PR introduce a breaking change?

No

Other information
No need of this statement I think

		if (!webpackConfigExists) {
			configPath = null;
			// end the generator stating webpack config not found or to specify the config
		}

Cause the below statement in modify-config-helper.ts will take care of that

	if (webpackConfigExists) {
			process.stdout.write(
				"\n" +
					logSymbols.success +
					chalk.green(" SUCCESS ") +
					"Found config " +
					chalk.cyan(configFile + "\n") +
					"\n"
			);

		} else {
			process.stdout.write(
				"\n" +
					logSymbols.error +
					chalk.red(" ERROR ") +
					chalk.cyan(configFile) +
					" not found. Please specify a valid path to your webpack config like \n " +
					chalk.white("$ ") +
					chalk.cyan(`webpack-cli ${action} webpack.dev.js`) +
					"\n"
			);
			return;
		}

Passing {configFile:configFile} to the env.run(generator) as second argument in packages/utils/modify-config.ts and using this property in generator class to get the configPath in generator class constructor in remove-generator.ts

webpack-bot · 2019-05-12T11:45:00Z

Thank you for your pull request! The most important CI builds succeeded, we’ll review the pull request soon.

anikethsaha · 2019-05-12T11:48:26Z

packages/generators/remove-generator.ts

 		const webpackConfigExists = fs.existsSync(configPath);
+
 		if (!webpackConfigExists) {


This checking is not required now. what do you think @ematipico ?

@misterdev what about this one ? should I remove this if statement?

Yeah you are right! We can remove it

misterdev

Hi, seems ok to me, I've left some comments about formatting

Thank you! :)

misterdev · 2019-05-12T19:40:50Z

packages/generators/remove-generator.ts

@@ -32,13 +32,15 @@ export default class RemoveGenerator extends Generator {
 				webpackOptions: {}
 			}
 		};
-
-		let configPath = path.resolve(process.cwd(), "webpack.config.js");
+		const {configFile} = opts


Add spaces and ;
const { configFile } = opts;

misterdev · 2019-05-12T19:45:38Z

packages/utils/modify-config-helper.ts

@@ -58,13 +58,14 @@ export default function modifyHelperUtil(
 					chalk.cyan(configFile + "\n") +
 					"\n"
 			);
+


Why this empty line? :)

removed it 👍

misterdev · 2019-05-12T19:46:58Z

packages/utils/modify-config-helper.ts

@@ -89,8 +90,9 @@ export default function modifyHelperUtil(
 	}
 	env.registerStub(generator, generatorName);

-	env.run(generatorName)
-		.then(
+	env.run(generatorName,{


Add space

env.run(generatorName, {

added semicolon in line 35

removed the blank line and added space in env.run method 's second argument.

anikethsaha · 2019-05-12T20:25:45Z

last two commits, 0b65ba8 and e007c45 are chore not fix , my bad :)

ematipico

Some feedback

ematipico · 2019-05-13T08:32:37Z

packages/generators/remove-generator.ts

@@ -32,13 +32,15 @@ export default class RemoveGenerator extends Generator {
 				webpackOptions: {}
 			}
 		};
-
-		let configPath = path.resolve(process.cwd(), "webpack.config.js");
+		const {configFile} = opts;


Suggested change

const {configFile} = opts;

const { configFile } = opts;

ematipico · 2019-05-13T08:33:23Z

packages/generators/remove-generator.ts

 		const webpackConfigExists = fs.existsSync(configPath);
+
 		if (!webpackConfigExists) {


Yeah you are right! We can remove it

ematipico · 2019-05-13T08:33:42Z

packages/utils/modify-config-helper.ts

-	env.run(generatorName)
-		.then(
+	env.run(generatorName, {
+		configFile


What's this change for?

To pass the config file name to the generator.

This change is probably ok and needed as we need this to be dynamic. Just gotta check that it works. Change is that the generator will have output of the config file supplied vs webpack.config.js

yes, I used this in my machine and its working fine. This can use workable with the other generators too like the update one and in add too

removed the if-statement and added space

anikethsaha · 2019-05-13T08:42:17Z

@ematipico Please review it again, I made those clean ups

Changed the init-generator code to fix the entry point issue when the entry point was answered

…ethsaha/webpack-cli into bugfix/init-entry-point-fix

misterdev

LGTM :)

anshumanv

Neat ✨

ematipico · 2019-05-22T08:50:55Z

Please rebase, we removed node 6 from the CI

Changed the init-generator code to fix the entry point issue when the entry point was answered

…-point-fix

…ethsaha/webpack-cli into bugfix/init-entry-point-fix

…b.com/anikethsaha/webpack-cli into bugfix/configPath-in-remove-generator

webpack-bot · 2019-05-23T19:03:44Z

@anikethsaha Thanks for your update.

I labeled the Pull Request so reviewers will review it again.

@anshumanv Please review the new changes.

anikethsaha · 2019-05-24T06:06:14Z

PTAL @ematipico

fix(generator): using configFile in configPath

2ca57a2

Passing {configFile:configFile} to the env.run(generator) as second argument in packages/utils/modify-config.ts and using this property in generator class to get the configPath in generator class constructor in remove-generator.ts

webpack-bot added PR: unreviewed PR: small labels May 12, 2019

webpack-bot added the PR: CI-ok label May 12, 2019

anikethsaha commented May 12, 2019

View reviewed changes

misterdev suggested changes May 12, 2019

View reviewed changes

webpack-bot added PR: reviewed-changes-requested and removed PR: unreviewed labels May 12, 2019

fix(lint): added semicolon

0b65ba8

added semicolon in line 35

webpack-bot added PR: review-outdated and removed PR: reviewed-changes-requested labels May 12, 2019

fix(lint): remove blank line and added space

e007c45

removed the blank line and added space in env.run method 's second argument.

ematipico reviewed May 13, 2019

View reviewed changes

chore: removed the if-statement

c395c16

removed the if-statement and added space

ematipico approved these changes May 16, 2019

View reviewed changes

webpack-bot added PR: reviewed-approved and removed PR: review-outdated labels May 16, 2019

anikethsaha added 2 commits May 21, 2019 16:37

fix(init): refractor the init-generator to fix entry issue

118ac1d

Changed the init-generator code to fix the entry point issue when the entry point was answered

Merge branch 'bugfix/init-entry-point-fix' of https://github.com/anik…

0e762d7

…ethsaha/webpack-cli into bugfix/init-entry-point-fix

anikethsaha mentioned this pull request May 21, 2019

Fix(init): init entry point fix when path is mentioned #886

Closed

misterdev approved these changes May 21, 2019

View reviewed changes

anshumanv approved these changes May 21, 2019

View reviewed changes

anikethsaha added 4 commits May 24, 2019 00:15

fix(init): refractor the init-generator to fix entry issue

04e7b17

Changed the init-generator code to fix the entry point issue when the entry point was answered

Merge remote-tracking branch 'upstream/master' into bugfix/init-entry…

7dc7a74

…-point-fix

Merge branch 'bugfix/init-entry-point-fix' of https://github.com/anik…

f8ddc41

…ethsaha/webpack-cli into bugfix/init-entry-point-fix

Merge branch 'bugfix/configPath-in-remove-generator' of https://githu…

737934b

…b.com/anikethsaha/webpack-cli into bugfix/configPath-in-remove-generator

webpack-bot added PR: review-outdated and removed PR: reviewed-approved labels May 23, 2019

ematipico merged commit fe31e65 into webpack:master May 24, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(generator): using configFile in configPath to get the config file name #883

fix(generator): using configFile in configPath to get the config file name #883

anikethsaha commented May 12, 2019 •

edited

Loading

webpack-bot commented May 12, 2019

anikethsaha May 12, 2019 •

edited

Loading

anikethsaha May 12, 2019

ematipico May 13, 2019

misterdev left a comment

misterdev May 12, 2019

anikethsaha May 12, 2019

misterdev May 12, 2019

anikethsaha May 12, 2019

misterdev May 12, 2019

anikethsaha May 12, 2019

anikethsaha commented May 12, 2019

ematipico left a comment

ematipico May 13, 2019

anikethsaha May 13, 2019

ematipico May 13, 2019

ematipico May 13, 2019

anikethsaha May 13, 2019

evenstensberg May 15, 2019

anikethsaha May 15, 2019

anikethsaha commented May 13, 2019

misterdev left a comment

anshumanv left a comment

ematipico commented May 22, 2019

webpack-bot commented May 23, 2019

anikethsaha commented May 24, 2019

		const webpackConfigExists = fs.existsSync(configPath);

		if (!webpackConfigExists) {

fix(generator): using configFile in configPath to get the config file name #883

fix(generator): using configFile in configPath to get the config file name #883

Conversation

anikethsaha commented May 12, 2019 • edited Loading

webpack-bot commented May 12, 2019

anikethsaha May 12, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

misterdev left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

anikethsaha commented May 12, 2019

ematipico left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

anikethsaha commented May 13, 2019

misterdev left a comment

Choose a reason for hiding this comment

anshumanv left a comment

Choose a reason for hiding this comment

ematipico commented May 22, 2019

webpack-bot commented May 23, 2019

anikethsaha commented May 24, 2019

anikethsaha commented May 12, 2019 •

edited

Loading

anikethsaha May 12, 2019 •

edited

Loading