Feature/generate loader #183

ianjsikes · 2017-09-05T00:41:13Z

What kind of change does this PR introduce?

Feature

Did you add tests for your changes?
A few

If relevant, did you update the documentation?

Summary

See #179 for feature request.
Adds two new commands, webpack-cli generate-loader and webpack-cli generate-plugin. These commands generate webpack-defaults-compliant starter projects for Webpack loaders and plugins, including basic tests.

Does this PR introduce a breaking change?

No

Other information
The starter projects are based off of @TheLarkInn's example loader repo.

jsf-clabot · 2017-09-05T00:41:14Z

All committers have signed the CLA.

evenstensberg · 2017-09-05T08:10:31Z

Heyo! Thanks for submitting a PR, will review later this week! Awesome work! CC-ing @okonet

TheLarkInn · 2017-09-05T14:14:25Z

@ianjsikes this looks incredible. Thank you so much for this work. This will be incredibly impactful on the webpack community. 🙇🙇🙇

okonet · 2017-09-05T15:04:59Z

That's great! @ianjsikes do you think it would make sense to add tests for the generator?

ianjsikes · 2017-09-05T17:28:54Z

@okonet definitely. I'll get started on that.

ianjsikes · 2017-09-06T06:31:15Z

So in attempting to add tests for the generator, I discovered that anything extending yeoman-generator will not work on node v4 or v5 (including the existing webpack-cli init generator) since yeoman-generator is written and distributed in ES6.

I can't figure out a way to write generator tests that will pass on v4 or v5 since yeoman-generator doesn't seem to work on those versions.

okonet · 2017-09-06T12:08:42Z

Does this mean that the webpack init will also not work on Node v4? This is a blocker then and we need to transpile the code in that case.

evenstensberg · 2017-09-08T07:27:06Z

bin/webpack.js

@@ -167,6 +167,10 @@ if(argv._.includes('init')) {
 	const inputConfigPath = path.resolve(process.cwd(), filePaths[0]);

 	return require('../lib/migrate.js')(inputConfigPath, inputConfigPath);
+} else if(argv._.includes('generate-loader')) {


If we were to support v4 this would be a thing that needs change. My self included, have used .includes, but I think that's v4<

evenstensberg

Well written. One thing that could be changed, is that some of the paths ( destinationPath, for instance ) could be written to a variable instead and reused.

evenstensberg · 2017-09-08T07:27:55Z

lib/generate-loader/loader-generator.js

+	}
+
+	writing() {
+		/**


Single line comment

evenstensberg · 2017-09-08T07:28:12Z

lib/generate-loader/loader-generator.js

+			this.destinationPath('src/cjs.js')
+		);
+
+		/**


Single line comment

evenstensberg · 2017-09-08T07:28:21Z

lib/generate-loader/loader-generator.js

+		);
+
+		/**
+		 * Examples


Single line comment

evenstensberg · 2017-09-08T07:29:27Z

lib/generate-loader/templates/test/unit.test.js.tpl

@@ -0,0 +1,32 @@
+import fs from 'fs';
+import { runLoaders } from 'loader-runner';
+import Promise from 'bluebird';


This should be at 2nd line

…I arg parsing

ianjsikes · 2017-09-08T23:51:41Z

I made the changes @ev1stensberg requested and opened #184 for fixing the yeoman-generator issue.

evenstensberg

Left some comments. The plugin and loader has a lot of the same logic. Maybe we could abstract that so that we could reuse some of the logic instead of re-writing it? Also, sorry for being strict, your PR is awesome, just need to make sure everything is top notch!

evenstensberg · 2017-09-09T07:53:49Z

bin/webpack.js

@@ -155,18 +155,22 @@ if(argv.verbose) {
 	argv['display-cached'] = true;
 	argv['display-cached-assets'] = true;
 }
-if(argv._.includes('init')) {
+if(argv._.indexOf('init') > -1) {


Prefer indexOf('init') >= 0

evenstensberg · 2017-09-09T07:54:09Z

bin/webpack.js

 	const initPkgs = argv._.length === 1 ? [] : [argv._.pop()];

 	return require('../lib/initialize')(initPkgs);
-} else if(argv._.includes('migrate')) {
+} else if(argv._.indexOf('migrate') > -1) {


indexOf('migrate') >= 0

evenstensberg · 2017-09-09T07:54:36Z

bin/webpack.js

 	const filePaths = argv._.length === 1 ? [] : [argv._.pop()];
 	if (!filePaths.length) {
 		throw new Error('Please specify a path to your webpack config');
 	}
 	const inputConfigPath = path.resolve(process.cwd(), filePaths[0]);

 	return require('../lib/migrate.js')(inputConfigPath, inputConfigPath);
+} else if(argv._.indexOf('generate-loader') > -1) {


indexOf('yourVal') >= 0

evenstensberg · 2017-09-09T07:55:58Z

lib/generate-loader/index.js

+const yeoman = require('yeoman-environment');
+const LoaderGenerator = require('./loader-generator').LoaderGenerator;
+
+/**


/* * Pad * Pad */

Without the double asterisks (/**), it isn't recognized as a JSDoc comment. I could still remove it but you won't get any code hints from your editor.

No, don't remove it. We should follow JSDoc everywhere. ESLint should be setup to enforce that.

Weird, we've used it like the one I've proposed, may need to change everything then?

Yeah, we should change it everywhere then.

evenstensberg · 2017-09-09T07:59:09Z

lib/generate-loader/loader-generator.js

+	}
+
+	default() {
+		if (path.basename(this.destinationPath()) !== this.props.name) {


Store this in a variable

evenstensberg · 2017-09-09T08:01:16Z

lib/generate-loader/loader-generator.js

+				'\nI will create this folder for you.'
+			);
+			mkdirp(this.props.name);
+			this.destinationRoot(this.destinationPath(this.props.name));


Store destinationPath(this.props.name) in a variable for new contributors to easier know what it is

evenstensberg · 2017-09-09T08:02:05Z

lib/generate-loader/loader-generator.js

+	}
+
+	writing() {
+		const copy = (filePath) => {


Leave a comment/ description of what this is doing

evenstensberg · 2017-09-09T08:03:04Z

lib/generate-plugin/plugin-generator.js

+	}
+
+	default() {
+		if (path.basename(this.destinationPath()) !== this.props.name) {


Use local var instead

evenstensberg · 2017-09-09T08:03:22Z

lib/generate-plugin/plugin-generator.js

+	}
+
+	writing() {
+		const copy = (filePath) => {


Maybe abstract this into a util file so you'd avoid re-declaring the same function twice, if it contains the same logic

…or class

ianjsikes · 2017-09-26T19:21:38Z

@okonet @ev1stensberg is there anything else that needs to be done on this PR?

evenstensberg · 2017-09-29T19:46:12Z

I'll have a final look and merge if it looks fine @ianjsikes !

evenstensberg · 2017-09-29T19:58:49Z

Remove the //eslint-disable's and you're set. I don't believe we'd need to worry about that when initializing a project, because we do not know their eslint config settings

…s/webpack-cli into feature/generate-loader

evenstensberg · 2017-09-29T21:47:19Z

LGTM, let's wait for tests and then merge if everything checks out

evenstensberg · 2017-09-29T21:59:44Z

Thanks @ianjsikes ❤️

* add unit tests for everything and flow compilation * add travis builds * add cli to signature & add ignore to trash code * port to jest * remove redundant cli command * Feature/generate loader (#183) * Add template files for loader yeoman generator * Create yeoman generator for a webpack loader project * Add tests for loader-generator * Add `mkdirp` dependency for loader-generator * Add function to create yeoman env and run loader-generator * Add `generate-loader` command to webpack-cli * Copy loader templates from proper directory * Add template files for plugin generator * Create yeoman generator for webpack plugins * Add function to create yeoman env and run plugin generator * Add cli command to generate plugin * Register generate- commands in yargs * Add template files for loader examples and tests * Copy loader test and example template files in generator * Add template files for plugin examples and tests * Copy plugin test and example template files in generator * Refactor generator file copying, switch .includes with .indexOf in CLI arg parsing * Change `indexOf('foo') > -1` to `indexOf('foo') >= 0` * Factor out generator copy utilities into separate module * Rewrite generators using a function that returns a customized generator class * Fix linting errors * Remove //eslint-disable lines from template files * add unit tests for everything and flow compilation * add travis builds * add cli to signature & add ignore to trash code * port to jest * remove redundant cli command * rebase against master

* Add template files for loader yeoman generator * Create yeoman generator for a webpack loader project * Add tests for loader-generator * Add `mkdirp` dependency for loader-generator * Add function to create yeoman env and run loader-generator * Add `generate-loader` command to webpack-cli * Copy loader templates from proper directory * Add template files for plugin generator * Create yeoman generator for webpack plugins * Add function to create yeoman env and run plugin generator * Add cli command to generate plugin * Register generate- commands in yargs * Add template files for loader examples and tests * Copy loader test and example template files in generator * Add template files for plugin examples and tests * Copy plugin test and example template files in generator * Refactor generator file copying, switch .includes with .indexOf in CLI arg parsing * Change `indexOf('foo') > -1` to `indexOf('foo') >= 0` * Factor out generator copy utilities into separate module * Rewrite generators using a function that returns a customized generator class * Fix linting errors * Remove //eslint-disable lines from template files

* add unit tests for everything and flow compilation * add travis builds * add cli to signature & add ignore to trash code * port to jest * remove redundant cli command * Feature/generate loader (#183) * Add template files for loader yeoman generator * Create yeoman generator for a webpack loader project * Add tests for loader-generator * Add `mkdirp` dependency for loader-generator * Add function to create yeoman env and run loader-generator * Add `generate-loader` command to webpack-cli * Copy loader templates from proper directory * Add template files for plugin generator * Create yeoman generator for webpack plugins * Add function to create yeoman env and run plugin generator * Add cli command to generate plugin * Register generate- commands in yargs * Add template files for loader examples and tests * Copy loader test and example template files in generator * Add template files for plugin examples and tests * Copy plugin test and example template files in generator * Refactor generator file copying, switch .includes with .indexOf in CLI arg parsing * Change `indexOf('foo') > -1` to `indexOf('foo') >= 0` * Factor out generator copy utilities into separate module * Rewrite generators using a function that returns a customized generator class * Fix linting errors * Remove //eslint-disable lines from template files * add unit tests for everything and flow compilation * add travis builds * add cli to signature & add ignore to trash code * port to jest * remove redundant cli command * rebase against master

* chore: Use prettier (#173) * build: Add webpack lint settings and style code with eslint --fix We now use the webpack eslint settings that make sense in the cli. Also use the new --fix of eslint to automatically beautify the code * Add additional .eslintrc to /bin to remove unrelated eslint errors * build: Add jest-cli as dev dependency * style: fix lintin errors and coding style according to webpack settings * build: update dependencies and add package-lock to repo * feat: Require quotes only in property names that require it * chore: Add again removed settings by merge conflict solving * tests: Fix failing test * chore: Use husky instead of pre-commit pre-commit package stopped working. See observing/pre-commit#113 husky also requires less boilerplate in the package.json * [Feature] Added support to Flow inside transformations (#174) * [FLOW] Implemented interfaces and flow types inside the transformations * [FLOW] first commit * Added flwo to two other files * Reviewed interfaces * Created different intefaces for expressions * Fixed rest of trasformations * More developments * Updated code after code review * Fixed failing tests * Removed yarn file * Applied linting * Updated packages * Applied lint style to the imports due to code review * docs: Documentation for migrate (#175) * initial draft of migrate documentation * formatting * formatting * added summary of migrate changes * maintain consistent usage of user through documentation * Feature/generate loader (#183) * Add template files for loader yeoman generator * Create yeoman generator for a webpack loader project * Add tests for loader-generator * Add `mkdirp` dependency for loader-generator * Add function to create yeoman env and run loader-generator * Add `generate-loader` command to webpack-cli * Copy loader templates from proper directory * Add template files for plugin generator * Create yeoman generator for webpack plugins * Add function to create yeoman env and run plugin generator * Add cli command to generate plugin * Register generate- commands in yargs * Add template files for loader examples and tests * Copy loader test and example template files in generator * Add template files for plugin examples and tests * Copy plugin test and example template files in generator * Refactor generator file copying, switch .includes with .indexOf in CLI arg parsing * Change `indexOf('foo') > -1` to `indexOf('foo') >= 0` * Factor out generator copy utilities into separate module * Rewrite generators using a function that returns a customized generator class * Fix linting errors * Remove //eslint-disable lines from template files * Add unit tests for CLI and add flow compilation (#186) * add unit tests for everything and flow compilation * add travis builds * add cli to signature & add ignore to trash code * port to jest * remove redundant cli command * Feature/generate loader (#183) * Add template files for loader yeoman generator * Create yeoman generator for a webpack loader project * Add tests for loader-generator * Add `mkdirp` dependency for loader-generator * Add function to create yeoman env and run loader-generator * Add `generate-loader` command to webpack-cli * Copy loader templates from proper directory * Add template files for plugin generator * Create yeoman generator for webpack plugins * Add function to create yeoman env and run plugin generator * Add cli command to generate plugin * Register generate- commands in yargs * Add template files for loader examples and tests * Copy loader test and example template files in generator * Add template files for plugin examples and tests * Copy plugin test and example template files in generator * Refactor generator file copying, switch .includes with .indexOf in CLI arg parsing * Change `indexOf('foo') > -1` to `indexOf('foo') >= 0` * Factor out generator copy utilities into separate module * Rewrite generators using a function that returns a customized generator class * Fix linting errors * Remove //eslint-disable lines from template files * add unit tests for everything and flow compilation * add travis builds * add cli to signature & add ignore to trash code * port to jest * remove redundant cli command * rebase against master * ast for devServer (#185) * feat: replace yarn backup w/ normal yarn (#160) * feat: replace yarn backup w/ normal yarn * upgrade npm & remove yarn * Use yarn if available (#189) * bump version to 1.3.4 * update package.json * 1.3.5 * prepublish -> postinstall * 1.3.6 * temp fix to prepublish issue * 1.3.7 * add propper post install * 1.3.8 * re-add manual build step * 1.3.9 * refactor * bump version * disable published pkg * add ignore patterns * add more ignore patterns for npm

ianjsikes added 16 commits September 4, 2017 13:06

Add template files for loader yeoman generator

63b3660

Create yeoman generator for a webpack loader project

14701f8

Add tests for loader-generator

4349ba7

Add mkdirp dependency for loader-generator

b633b14

Add function to create yeoman env and run loader-generator

9de21fa

Add generate-loader command to webpack-cli

b6972a7

Copy loader templates from proper directory

91ab4ea

Add template files for plugin generator

6a3e6a6

Create yeoman generator for webpack plugins

1aa6d30

Add function to create yeoman env and run plugin generator

57fa953

Add cli command to generate plugin

d550206

Register generate- commands in yargs

88eeede

Add template files for loader examples and tests

b73a24d

Copy loader test and example template files in generator

0cb2f69

Add template files for plugin examples and tests

7d6fd90

Copy plugin test and example template files in generator

4aa82f2

evenstensberg added enhancement Feature labels Sep 5, 2017

ianjsikes force-pushed the feature/generate-loader branch from 78349d7 to 4aa82f2 Compare September 6, 2017 06:17

evenstensberg reviewed Sep 8, 2017

View reviewed changes

evenstensberg requested changes Sep 8, 2017

View reviewed changes

ianjsikes mentioned this pull request Sep 8, 2017

webpack-cli init does not work on < Node v6 #184

Closed

Refactor generator file copying, switch .includes with .indexOf in CL…

bb164a8

…I arg parsing

evenstensberg requested changes Sep 9, 2017

View reviewed changes

ianjsikes added 3 commits September 10, 2017 19:40

Change indexOf('foo') > -1 to indexOf('foo') >= 0

10ce3c2

Factor out generator copy utilities into separate module

4a8f738

Rewrite generators using a function that returns a customized generat…

49c887e

…or class

evenstensberg approved these changes Sep 11, 2017

View reviewed changes

ianjsikes added 3 commits September 14, 2017 22:22

Merge branch 'master' into feature/generate-loader

e93c842

Fix linting errors

a106d9d

Merge branch 'master' into feature/generate-loader

09aef62

ianjsikes added 2 commits September 29, 2017 14:42

Remove //eslint-disable lines from template files

41e2c9e

Merge branch 'feature/generate-loader' of https://github.com/ianjsike…

5930f37

…s/webpack-cli into feature/generate-loader

evenstensberg merged commit e887dba into webpack:master Sep 29, 2017

ianjsikes deleted the feature/generate-loader branch September 29, 2017 22:01

evenstensberg mentioned this pull request Dec 15, 2017

Type Error: argv._.includes('init') in Node 4.4 #140

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/generate loader #183

Feature/generate loader #183

ianjsikes commented Sep 5, 2017

jsf-clabot commented Sep 5, 2017 •

edited

Loading

evenstensberg commented Sep 5, 2017

TheLarkInn commented Sep 5, 2017

okonet commented Sep 5, 2017

ianjsikes commented Sep 5, 2017

ianjsikes commented Sep 6, 2017

okonet commented Sep 6, 2017

evenstensberg Sep 8, 2017

evenstensberg left a comment

evenstensberg Sep 8, 2017

evenstensberg Sep 8, 2017

evenstensberg Sep 8, 2017

evenstensberg Sep 8, 2017

ianjsikes commented Sep 8, 2017

evenstensberg left a comment

evenstensberg Sep 9, 2017

evenstensberg Sep 9, 2017

evenstensberg Sep 9, 2017

evenstensberg Sep 9, 2017

ianjsikes Sep 10, 2017

okonet Sep 11, 2017

evenstensberg Sep 11, 2017

okonet Sep 11, 2017

evenstensberg Sep 9, 2017

evenstensberg Sep 9, 2017

evenstensberg Sep 9, 2017

evenstensberg Sep 9, 2017

evenstensberg Sep 9, 2017

ianjsikes commented Sep 26, 2017

evenstensberg commented Sep 29, 2017

evenstensberg commented Sep 29, 2017

evenstensberg commented Sep 29, 2017

evenstensberg commented Sep 29, 2017

Feature/generate loader #183

Feature/generate loader #183

Conversation

ianjsikes commented Sep 5, 2017

jsf-clabot commented Sep 5, 2017 • edited Loading

evenstensberg commented Sep 5, 2017

TheLarkInn commented Sep 5, 2017

okonet commented Sep 5, 2017

ianjsikes commented Sep 5, 2017

ianjsikes commented Sep 6, 2017

okonet commented Sep 6, 2017

Choose a reason for hiding this comment

evenstensberg 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

ianjsikes commented Sep 8, 2017

evenstensberg 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

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

ianjsikes commented Sep 26, 2017

evenstensberg commented Sep 29, 2017

evenstensberg commented Sep 29, 2017

evenstensberg commented Sep 29, 2017

evenstensberg commented Sep 29, 2017

jsf-clabot commented Sep 5, 2017 •

edited

Loading