Webpack parser and rule #957

sarvaje · 2018-03-30T16:49:54Z

Pull request checklist

Make sure you:

Signed the Contributor License Agreement
Followed the commit message guidelines

For non-trivial changes, please make sure you also:

Added/Updated related documentation.
Added/Updated related tests.

Short description of the change(s)

alrra · 2018-03-30T21:36:33Z

.eslintignore

@@ -1,3 +1,5 @@
 coverage
 dist
 node_modules
+fixtures
+packages/rule-webpack-config/README.md


Why is ESLint looking at this file?

Eslint look at all the md files and check the js/ts code.
In this case, I have an intentional error in the code, and I don't want Eslint to fail :)

alrra · 2018-04-02T16:03:02Z

.eslintignore

@@ -1,3 +1,5 @@
 coverage
 dist
 node_modules
+fixtures
+packages/rule-webpack-config/docs/is-valid.md


@sarvaje Can this be moved in a .eslintignore file in packages/rule-webpack-config/?

I'm going to try it

We can't do that, we are using only this .eslintignore

And if you create another .eslintignore in there it doesn't work?

I think the only thing we can do is copy/paste this file to the folder rule-webpack-config and modify the package.json script to use that file instead of the one in the root folder.

That's weird, usually ESLint uses the .eslintrc closest to the resource in combination with the one in the root so I would expect it to do the same with .eslintignore.

alrra · 2018-04-02T16:04:07Z

packages/parser-webpack-config/README.md

@@ -0,0 +1,71 @@
+# Parser webpack-config (`@sonarwhal/parser-webpack-config`)


Maybe use

webpack config parser

alrra · 2018-04-02T16:06:01Z

packages/parser-webpack-config/README.md

+# Parser webpack-config (`@sonarwhal/parser-webpack-config`)
+
+The `webpack-config` parser allows the user to analyze the
+Webpack configuration in their projects.


webpack

(It seems they use webpack instead of Webpack)

Can you make this change everywhere? Thanks!

alrra · 2018-04-02T16:09:22Z

packages/rule-webpack-config/README.md

@@ -0,0 +1,63 @@
+<!-- markdownlint-disable MD024 -->


alrra · 2018-04-02T16:10:15Z

packages/rule-webpack-config/README.md

@@ -0,0 +1,63 @@
+<!-- markdownlint-disable MD024 -->
+
+# webpack-config (`@sonarwhal/rule-webpack-config`)


webpack-config

Maybe use a more descriptive title?

sarvaje · 2018-04-02T18:23:13Z

@alrra this should be done!

molant

First revision, mostly typos. Need to take a deeper look later.

molant · 2018-04-05T20:12:14Z

.eslintignore

@@ -1,3 +1,5 @@
 coverage
 dist
 node_modules
+fixtures
+packages/rule-webpack-config/docs/is-valid.md


And if you create another .eslintignore in there it doesn't work?

molant · 2018-04-05T20:35:54Z

packages/rule-webpack-config/docs/module-esnext-typescript.md

+### Examples that **pass** the rule
+
+`typescript-config` configured and `complierOptions.module` has
+a the value `esnext`


molant · 2018-04-05T20:37:11Z

packages/rule-webpack-config/src/is-valid.ts

@@ -0,0 +1,43 @@
+/**
+ * @fileoverview `webpack-config/is-valid` warns again providing an invalid webpack configuration file `webpack.config.js`.


molant · 2018-04-05T20:38:59Z

packages/rule-webpack-config/src/no-devtool-in-prod.ts

@@ -0,0 +1,45 @@
+/**
+ * @fileoverview `webpack-config/no-devtool-in-prod` warns again having set the propety `devtool` to `eval`.


molant · 2018-04-05T20:39:02Z

packages/rule-webpack-config/src/no-devtool-in-prod.ts

+    public static readonly meta: RuleMetadata = {
+        docs: {
+            category: Category.development,
+            description: '`webpack-config/no-devtool-in-prod` warns again having set the propety `devtool` to `eval`'


molant · 2018-04-05T20:40:06Z

packages/rule-webpack-config/src/is-installed.ts

+    public static readonly meta: RuleMetadata = {
+        docs: {
+            category: Category.development,
+            description: '`webpack-config/is-installed` warns again not having webpack installed'


molant · 2018-04-05T20:40:17Z

packages/rule-webpack-config/src/is-valid.ts

+    public static readonly meta: RuleMetadata = {
+        docs: {
+            category: Category.development,
+            description: '`webpack-config/is-valid` warns again providing an invalid webpack configuration file `webpack.config.js`'


molant · 2018-04-05T20:40:32Z

packages/rule-webpack-config/src/module-esnext-typescript.ts

@@ -0,0 +1,77 @@
+/**
+ * @fileoverview `webpack-config/module-esnext-typescript` warns again not having set the propety `compilerOptions.module` to `esnext` in typescript configuration file.


molant · 2018-04-05T20:40:45Z

packages/rule-webpack-config/src/module-esnext-typescript.ts

+    public static readonly meta: RuleMetadata = {
+        docs: {
+            category: Category.development,
+            description: '`webpack-config/module-esnext-typescript` warns again not having set the propety `compilerOptions.module` to `esnext` in typescript configuration file'


molant · 2018-04-05T20:40:55Z

packages/rule-webpack-config/src/modules-false-babel.ts

@@ -0,0 +1,82 @@
+/**
+ * @fileoverview `webpack-config/modules-false-babel` warns again not having set the propety `modules` to `false` in presets in babel configuration file.


sarvaje · 2018-04-06T16:03:40Z

@molant done

alrra · 2018-04-08T00:56:37Z

packages/parser-webpack-config/README.md

+
+## Types
+
+If you need to import any type defined in this parser, you just need to


Remove just.

(see: #939)

alrra · 2018-04-08T00:59:08Z

packages/parser-webpack-config/README.md

+  package `webpack` is not installed locally.
+  This event doesn't containt anything else.
+
+* `parse::webpack-config::error::not-found`. This event is sent if the


Same observation as in #973 (comment).

(Cc @molant)

alrra · 2018-04-08T01:00:12Z

packages/parser-webpack-config/README.md

+  * `resource`: the parsed resource.
+  * `error`: the error emited parsing the configuration file.
+
+* `parse::webpack-config::error::not-install`. This event is sent if the


Should this be the job of the parser?

(Cc @molant)

@sarvaje You will do the changes in another PR, right?

alrra · 2018-04-08T01:01:23Z

packages/parser-webpack-config/package.json

+    "webpack-config"
+  ],
+  "license": "Apache-2.0",
+  "main": "dist/src/webpack-config.js",


Use parser.js.

alrra · 2018-04-08T01:04:49Z

packages/parser-webpack-config/src/webpack-config.ts

+        const resource = fetchEnd.resource;
+        const fileName = path.basename(resource);
+
+        if (fileName !== 'webpack.config.js') {


Is this the only place the configs can be placed in?

Can you provide a link for this? Thanks!

I didn't find any reference about another place to have the webpack config. Do you think I should add this link? https://webpack.js.org/configuration/

Works for me.

@alrra this is done!

alrra · 2018-04-08T01:07:54Z

packages/rule-webpack-config/docs/config-exists.md

@@ -0,0 +1,18 @@
+# `config-exists` (`webpack-config/config-exists`)


Maybe drop (webpack-config/config-exists)?

do you want me to do that in the other rules doc files?

sarvaje requested review from alrra and molant as code owners March 30, 2018 16:49

alrra reviewed Mar 30, 2018

View reviewed changes

webhintio deleted a comment from sarvaje Mar 31, 2018

alrra reviewed Apr 2, 2018

View reviewed changes

alrra force-pushed the master branch from 25bffb8 to d2009c4 Compare April 3, 2018 12:29

molant requested changes Apr 5, 2018

View reviewed changes

alrra reviewed Apr 8, 2018

View reviewed changes

sarvaje force-pushed the webpack-parser branch 3 times, most recently from c77e951 to 3b2a2fe Compare April 10, 2018 16:28

alrra force-pushed the master branch from dfad3e7 to 6ed6c97 Compare April 10, 2018 18:50

sarvaje force-pushed the webpack-parser branch 2 times, most recently from 3671da3 to 3d388bc Compare April 10, 2018 19:54

sarvaje added 6 commits April 10, 2018 13:14

New: Add after/before support to local tests runner

78893e7

Fix: Typo in rule description

b3af27f

New: Add webpack config parser

a761aaa

New: Add webpack config rules

b587c35

Docs: Update user guide rule list

06d3869

Chore: Exclude webpack parser and rules from the release script

1f66efa

sarvaje force-pushed the webpack-parser branch from 3d388bc to 1f66efa Compare April 10, 2018 20:18

alrra closed this in df50c5f Apr 10, 2018

sarvaje deleted the webpack-parser branch May 10, 2018 23:14

		@@ -0,0 +1,71 @@
		# Parser webpack-config (`@sonarwhal/parser-webpack-config`)

		@@ -0,0 +1,63 @@
		<!-- markdownlint-disable MD024 -->

		# webpack-config (`@sonarwhal/rule-webpack-config`)

		@@ -0,0 +1,43 @@
		/**
		* @fileoverview `webpack-config/is-valid` warns again providing an invalid webpack configuration file `webpack.config.js`.

		@@ -0,0 +1,45 @@
		/**
		* @fileoverview `webpack-config/no-devtool-in-prod` warns again having set the propety `devtool` to `eval`.

		@@ -0,0 +1,77 @@
		/**
		* @fileoverview `webpack-config/module-esnext-typescript` warns again not having set the propety `compilerOptions.module` to `esnext` in typescript configuration file.

		@@ -0,0 +1,82 @@
		/**
		* @fileoverview `webpack-config/modules-false-babel` warns again not having set the propety `modules` to `false` in presets in babel configuration file.


		## Types

		If you need to import any type defined in this parser, you just need to

		@@ -0,0 +1,18 @@
		# `config-exists` (`webpack-config/config-exists`)

Webpack parser and rule #957

Webpack parser and rule #957

Conversation

sarvaje commented Mar 30, 2018 • edited Loading

Pull request checklist

Short description of the change(s)

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alrra Apr 2, 2018 • edited Loading

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

sarvaje commented Apr 2, 2018

molant 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

sarvaje commented Apr 6, 2018

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

sarvaje commented Mar 30, 2018 •

edited

Loading

alrra Apr 2, 2018 •

edited

Loading