add fs option by heygrady · Pull Request #1819 · webpack/webpack-dev-server

heygrady · 2019-04-25T06:32:39Z

add fs option, sort options file, correct links in error messages, fix validation test (#1818)

codecov · 2019-04-25T06:49:38Z

Codecov Report

Merging #1819 into master will increase coverage by 1.8%.
The diff coverage is n/a.

@@            Coverage Diff            @@
##           master    #1819     +/-   ##
=========================================
+ Coverage   87.66%   89.47%   +1.8%     
=========================================
  Files           9        9             
  Lines         608      608             
  Branches      185      185             
=========================================
+ Hits          533      544     +11     
+ Misses         63       54      -9     
+ Partials       12       10      -2

Impacted Files	Coverage Δ
lib/utils/createConfig.js	`93.68% <ø> (ø)`	⬆️
lib/Server.js	`85.96% <0%> (+2.5%)`	⬆️
lib/utils/createLogger.js	`100% <0%> (+12.5%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5195bfa...74a0584. Read the comment docs.

alexander-akait · 2019-04-25T09:54:01Z

+        throw new Error("Validation failed and it shouldn't");
+      }
+    });
+  });


Move this to other test file (fs.test.js)

heygrady · 2019-04-25T16:50:12Z

For posterity, this is the script I used to organize the options.json file.

'use strict';

const fs = require('fs-extra');
const ucFirst = require('lodash.upperfirst');

const middlewareLinks = [
  'https://github.com/webpack/webpack-dev-middleware#methods',
  'https://github.com/webpack/webpack-dev-middleware#headers',
  'https://github.com/webpack/webpack-dev-middleware#index',
  'https://github.com/webpack/webpack-dev-middleware#lazy',
  'https://github.com/webpack/webpack-dev-middleware#logger',
  'https://github.com/webpack/webpack-dev-middleware#loglevel',
  'https://github.com/webpack/webpack-dev-middleware#logtime',
  'https://github.com/webpack/webpack-dev-middleware#mimetypes',
  'https://github.com/webpack/webpack-dev-middleware#publicpath',
  'https://github.com/webpack/webpack-dev-middleware#reporter',
  'https://github.com/webpack/webpack-dev-middleware#serversiderender',
  'https://github.com/webpack/webpack-dev-middleware#stats',
  'https://github.com/webpack/webpack-dev-middleware#watchoptions',
  'https://github.com/webpack/webpack-dev-middleware#writetodisk',
  'https://github.com/webpack/webpack-dev-middleware#fs',
];
const links = [
  'https://webpack.js.org/configuration/dev-server/#devserverafter',
  'https://webpack.js.org/configuration/dev-server/#devserverallowedhosts',
  'https://webpack.js.org/configuration/dev-server/#devserverbefore',
  'https://webpack.js.org/configuration/dev-server/#devserverbonjour',
  'https://webpack.js.org/configuration/dev-server/#devserverclientloglevel',
  'https://webpack.js.org/configuration/dev-server/#devservercolor---cli-only',
  'https://webpack.js.org/configuration/dev-server/#devservercompress',
  'https://webpack.js.org/configuration/dev-server/#devservercontentbase',
  'https://webpack.js.org/configuration/dev-server/#devserverdisablehostcheck',
  'https://webpack.js.org/configuration/dev-server/#devserverfilename-',
  'https://webpack.js.org/configuration/dev-server/#devserverheaders-',
  'https://webpack.js.org/configuration/dev-server/#devserverhistoryapifallback',
  'https://webpack.js.org/configuration/dev-server/#devserverhost',
  'https://webpack.js.org/configuration/dev-server/#devserverhot',
  'https://webpack.js.org/configuration/dev-server/#devserverhotonly',
  'https://webpack.js.org/configuration/dev-server/#devserverhttp2',
  'https://webpack.js.org/configuration/dev-server/#devserverhttps',
  'https://webpack.js.org/configuration/dev-server/#devserverindex',
  'https://webpack.js.org/configuration/dev-server/#devserverinfo---cli-only',
  'https://webpack.js.org/configuration/dev-server/#devserverinline',
  'https://webpack.js.org/configuration/dev-server/#devserverlazy-',
  'https://webpack.js.org/configuration/dev-server/#devservermimetypes-',
  'https://webpack.js.org/configuration/dev-server/#devservernoinfo-',
  'https://webpack.js.org/configuration/dev-server/#devserveropen',
  'https://webpack.js.org/configuration/dev-server/#devserveropenpage',
  'https://webpack.js.org/configuration/dev-server/#devserveroverlay',
  'https://webpack.js.org/configuration/dev-server/#devserverpfx',
  'https://webpack.js.org/configuration/dev-server/#devserverpfxpassphrase',
  'https://webpack.js.org/configuration/dev-server/#devserverport',
  'https://webpack.js.org/configuration/dev-server/#devserverproxy',
  'https://webpack.js.org/configuration/dev-server/#devserverprogress---cli-only',
  'https://webpack.js.org/configuration/dev-server/#devserverpublic',
  'https://webpack.js.org/configuration/dev-server/#devserverpublicpath-',
  'https://webpack.js.org/configuration/dev-server/#devserverquiet-',
  'https://webpack.js.org/configuration/dev-server/#devserverserveindex',
  'https://webpack.js.org/configuration/dev-server/#devserversetup',
  'https://webpack.js.org/configuration/dev-server/#devserversocket',
  'https://webpack.js.org/configuration/dev-server/#devserversockpath',
  'https://webpack.js.org/configuration/dev-server/#devserverstaticoptions',
  'https://webpack.js.org/configuration/dev-server/#devserverstats-',
  'https://webpack.js.org/configuration/dev-server/#devserverstdin---cli-only',
  'https://webpack.js.org/configuration/dev-server/#devserveruselocalip',
  'https://webpack.js.org/configuration/dev-server/#devserverwatchcontentbase',
  'https://webpack.js.org/configuration/dev-server/#devserverwatchoptions-',
  'https://webpack.js.org/configuration/dev-server/#devserverwritetodisk-',
];

const findPropLink = (prop) =>
  links.find((link) => link.includes(`#devserver${prop.toLowerCase()}`)) ||
  middlewareLinks.find((link) =>
    link.endsWith(`middleware#${prop.toLowerCase()}`)
  );

const fixOptions = async () => {
  const options = await fs.readJson('./lib/options.json');

  // regenerate error messages, marry them with the correct links
  const errorMessages = {};
  for (const [prop, value] of Object.entries(options.properties)) {
    let message = 'should be ';
    if (value.type || value.instanceof) {
      message += `{${ucFirst(value.type || value.instanceof)}}`;
    } else if (value.anyOf) {
      const type = value.anyOf
        .map((obj) => ucFirst(obj.type || obj.instanceof))
        .filter(Boolean)
        .join('|');
      message += `{${type}}`;
    } else if (value.enum) {
      const values = value.enum.map((string) => `'${string}'`).join(', ');
      message += `{String} and equal to one of the allowed values\n\n [ ${values} ]\n\n`;
    } else {
      console.error(
        `Did not handle ${prop}, unable to process value: ${JSON.stringify(
          value
        )}`
      );
    }

    const link = findPropLink(prop);
    if (link) {
      message += ` (${link})`;
    }
    errorMessages[prop] = message;
  }

  // sort the keys
  const originalProperties = options.properties;
  const keys = Object.keys(originalProperties).sort();
  options.properties = {};
  options.errorMessage.properties = {};
  for (const key of keys) {
    options.properties[key] = originalProperties[key];
    options.errorMessage.properties[key] = errorMessages[key];
  }
  await fs.writeJson('./lib/options.json', options, { spaces: '  ' });

  // identify webpack-dev-middleware options that are not in the options.json
  const lowerKeys = new Set(keys.map((key) => key.toLowerCase()));
  for (const link of middlewareLinks) {
    const [, prop] = link.split('#');
    if (!lowerKeys.has(prop)) {
      console.warn(
        `${prop} is a valid middleware option that is not in the options.json file.`
      );
    }
  }
};

fixOptions();

alexander-akait · 2019-04-25T16:54:01Z

+          throw err;
+        }
+
+        throw new Error("Validation failed and it shouldn't");


Each test should contain expect, let's move error (let error = null) on top and check expect(error).toBe(null)

alexander-akait

One note about test

alexander-akait

Good job!

hiroppy · 2019-04-25T17:55:21Z

IMO, I want to add options.json tests before merging.
@evilebottnawi What do you think?

alexander-akait · 2019-04-25T18:03:52Z

Yes let's do it 👍

alexander-akait · 2019-04-25T18:04:01Z

/cc @heygrady

hiroppy · 2019-04-26T02:32:17Z

I created options tests so please rebase this if it is merged.
The amount of change in this PR was too large to review so I want to compare the current master status snapshot with the latest status of this branch.

Thank you for adding options tests!

alexander-akait · 2019-04-26T10:13:34Z

/cc @heygrady hi, need rebase, thanks

heygrady · 2019-04-26T18:18:20Z

Rebased.

Some notes:

Many of the links in the error messages are inaccurate and I updated them, so I had to update the snapshot.
Some of the links were totally bogus for features that are not documented, those now have no links.
Some of the messages regarding supported types were inaccurate, and they have been updated.
I have done that in the last commit (98b5778) so if you wanted to test that separately you could. We could make that commit a separate PR if desired.

alexander-akait

AWESOME WORK!

alexander-akait · 2019-04-26T18:26:29Z

/cc @hiroppy

hiroppy

lgtm, thanks

heygrady requested review from alexander-akait and hiroppy as code owners April 25, 2019 06:32

alexander-akait reviewed Apr 25, 2019

View reviewed changes

alexander-akait requested changes Apr 25, 2019

View reviewed changes

alexander-akait approved these changes Apr 25, 2019

View reviewed changes

heygrady force-pushed the fs-option branch from 37ca847 to 45ab7c6 Compare April 25, 2019 20:09

hiroppy mentioned this pull request Apr 26, 2019

add options test #1821

Merged

6 tasks

heygrady added 3 commits April 26, 2019 10:50

add fs option, fix validation test (webpack#1818)

8cef3b0

adding some more tests

2b1721f

sort and normalize options

98b5778

heygrady force-pushed the fs-option branch from 9bdc56e to 98b5778 Compare April 26, 2019 18:13

alexander-akait approved these changes Apr 26, 2019

View reviewed changes

hiroppy approved these changes Apr 27, 2019

View reviewed changes

Merge branch 'master' into fs-option

74a0584

hiroppy merged commit b5bc05c into webpack:master Apr 27, 2019

hiroppy mentioned this pull request Apr 27, 2019

fs option not supported #1818

Closed

Uh oh!

Uh oh!

Conversation

heygrady commented Apr 25, 2019

Uh oh!

codecov Bot commented Apr 25, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

alexander-akait Apr 25, 2019

Choose a reason for hiding this comment

Uh oh!

heygrady commented Apr 25, 2019

Uh oh!

alexander-akait Apr 25, 2019

Choose a reason for hiding this comment

Uh oh!

alexander-akait left a comment

Choose a reason for hiding this comment

Uh oh!

alexander-akait left a comment

Choose a reason for hiding this comment

Uh oh!

hiroppy commented Apr 25, 2019

Uh oh!

alexander-akait commented Apr 25, 2019

Uh oh!

alexander-akait commented Apr 25, 2019

Uh oh!

hiroppy commented Apr 26, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alexander-akait commented Apr 26, 2019

Uh oh!

heygrady commented Apr 26, 2019

Uh oh!

alexander-akait left a comment

Choose a reason for hiding this comment

Uh oh!

alexander-akait commented Apr 26, 2019

Uh oh!

hiroppy left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

codecov Bot commented Apr 25, 2019 •

edited

Loading

hiroppy commented Apr 26, 2019 •

edited

Loading