improve Flow

frenic · May 2, 2019 · 16af68b · 16af68b
1 parent cb80556
commit 16af68b
Show file tree

Hide file tree

Showing 10 changed files with 2,509 additions and 1,795 deletions.
diff --git a/.flowconfig b/.flowconfig
@@ -1,12 +1,23 @@
 [ignore]
 .*/node_modules
+.*/__fixtures__/.*
 
 [include]
 
 [libs]
 
 [lints]
+untyped-import=warn
 
 [options]
 
 [strict]
+sketchy-null
+sketchy-number
+untyped-type-import
+unclear-type
+unsafe-getters-setters
+nonstrict-import
+unnecessary-optional-chain
+unnecessary-invariant
+deprecated-utility
diff --git a/__fixtures__/.flowconfig b/__fixtures__/.flowconfig
@@ -0,0 +1,22 @@
+[ignore]
+
+[include]
+../index.js.flow
+
+[libs]
+
+[lints]
+untyped-import=warn
+
+[options]
+
+[strict]
+sketchy-null
+sketchy-number
+untyped-type-import
+unclear-type
+unsafe-getters-setters
+nonstrict-import
+unnecessary-optional-chain
+unnecessary-invariant
+deprecated-utility
diff --git a/__fixtures__/flow-errors-fixture.js b/__fixtures__/flow-errors-fixture.js
@@ -0,0 +1,13 @@
+// @flow strict
+import * as CSS from '../index.js.flow';
+
+const css: CSS.Properties<*> = {
+  flexGrow: 'invalid',
+  unknownProperty: 'here',
+};
+
+const cssWithFallbackValues: CSS.PropertiesFallback<*> = {
+  flexGrow: [true],
+  flexDirection: ['123'],
+  colour: 'typo',
+};
diff --git a/__tests__/__snapshots__/flow.ts.snap b/__tests__/__snapshots__/flow.ts.snap
@@ -0,0 +1,77 @@
+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[`it detects errors 1`] = `
+"Error ------------------------------------------------------------------------- __fixtures__/flow-errors-fixture.js:4:32
+
+Cannot assign object literal to \`css\` because:
+ - property \`unknownProperty\` is missing in \`Properties\` [1] but exists in object literal [2].
+ - in property \`flexGrow\`:
+    - Either string [3] is incompatible with enum [4].
+    - Or string [3] is incompatible with number [5].
+
+   __fixtures__/flow-errors-fixture.js:4:32
+                                        v
+      4| const css: CSS.Properties<*> = {
+      5|   flexGrow: 'invalid',
+      6|   unknownProperty: 'here',
+      7| };
+         ^ [2]
+
+References:
+   __fixtures__/flow-errors-fixture.js:4:12
+      4| const css: CSS.Properties<*> = {
+                    ^^^^^^^^^^^^^^^^^ [1]
+   __fixtures__/flow-errors-fixture.js:5:13
+      5|   flexGrow: 'invalid',
+                     ^^^^^^^^^ [3]
+   index.js.flow:4555:22
+   4555| type GlobalsNumber = Globals | number;
+                              ^^^^^^^ [4]
+   index.js.flow:4555:32
+   4555| type GlobalsNumber = Globals | number;
+                                        ^^^^^^ [5]
+
+
+Error ------------------------------------------------------------------------- __fixtures__/flow-errors-fixture.js:9:58
+
+Cannot assign object literal to \`cssWithFallbackValues\` because:
+ - property \`colour\` is missing in \`PropertiesFallback\` [1] but exists in object literal [2].
+ - in array element of property \`flexGrow\`:
+    - Either boolean [3] is incompatible with enum [4].
+    - Or boolean [3] is incompatible with number [5].
+ - string [6] is incompatible with enum [7] in array element of property \`flexDirection\`.
+
+   __fixtures__/flow-errors-fixture.js:9:58
+                                                                  v
+      9| const cssWithFallbackValues: CSS.PropertiesFallback<*> = {
+     10|   flexGrow: [true],
+     11|   flexDirection: ['123'],
+     12|   colour: 'typo',
+     13| };
+         ^ [2]
+
+References:
+   __fixtures__/flow-errors-fixture.js:9:30
+      9| const cssWithFallbackValues: CSS.PropertiesFallback<*> = {
+                                      ^^^^^^^^^^^^^^^^^^^^^^^^^ [1]
+   __fixtures__/flow-errors-fixture.js:10:14
+     10|   flexGrow: [true],
+                      ^^^^ [3]
+   index.js.flow:4555:22
+   4555| type GlobalsNumber = Globals | number;
+                              ^^^^^^^ [4]
+   index.js.flow:4555:32
+   4555| type GlobalsNumber = Globals | number;
+                                        ^^^^^^ [5]
+   __fixtures__/flow-errors-fixture.js:11:19
+     11|   flexDirection: ['123'],
+                           ^^^^^ [6]
+   index.js.flow:106:72
+    106| type FallbackableFlexDirectionProperty = FlexDirectionProperty | Array<FlexDirectionProperty>;
+                                                                                ^^^^^^^^^^^^^^^^^^^^^ [7]
+
+
+
+Found 5 errors
+"
+`;
diff --git a/__tests__/flow.ts b/__tests__/flow.ts
@@ -0,0 +1,22 @@
+import * as path from 'path';
+import { spawnSync } from 'child_process';
+
+test('it detects errors', () => {
+  const flowBin = path.resolve(
+    __dirname,
+    '../node_modules',
+    '.bin',
+    process.platform === 'win32' ? 'flow.cmd' : 'flow',
+  );
+
+  const fixturesDir = path.resolve(__dirname, '../__fixtures__');
+
+  const args = ['check', fixturesDir];
+
+  const result = spawnSync(flowBin, args, {
+    stdio: 'pipe',
+    encoding: 'utf8',
+  });
+
+  expect(result.stdout).toMatchSnapshot();
+});
diff --git a/index.js.flow b/index.js.flow
diff --git a/package.json b/package.json
@@ -16,7 +16,7 @@
     "chalk": "^2.4.2",
     "chokidar": "^2.1.2",
     "fast-glob": "^2.2.6",
-    "flow-bin": "^0.94.0",
+    "flow-bin": "^0.98.0",
     "jest": "^24.1.0",
     "jsdom": "^13.2.0",
     "mdn-browser-compat-data": "git+https://github.com/mdn/browser-compat-data.git#cca8064adf09a9e95a8ff0d2abe233798912b5d0",

diff --git a/src/output.ts b/src/output.ts
@@ -8,44 +8,73 @@ export default () => ({
   typescript: typescript(),
 });
 
+function combineFlowExactTypes(input: string[]): string {
+  if (input.length === 0) {
+    return '';
+  } else if (input.length === 1) {
+    return input[0];
+  } else {
+    return '{|' + input.map(type => `...${type}`).join(`,${EOL}`) + '|}';
+  }
+}
+
+function getNameForFallbackable(name: string): string {
+  return 'Fallbackable' + name[0].toUpperCase() + name.slice(1);
+}
+
 function flow() {
   let interfacesOutput = '';
+
+  const fallbackSet: Set<string> = new Set();
+
   for (const item of interfaces) {
     if (interfacesOutput) {
       interfacesOutput += EOL + EOL;
     }
 
-    const extendList = item.extends.map(extend => extend.name + stringifyGenerics(extend.generics, true)).join(' & ');
+    const extendList = combineFlowExactTypes(
+      item.extends.map(extend => extend.name + stringifyGenerics(extend.generics, true)),
+    );
+
     interfacesOutput += 'export type ';
     interfacesOutput += item.name + stringifyGenerics(item.generics);
     interfacesOutput += ' = ' + extendList;
 
     if (item.properties.length > 0) {
       if (extendList) {
+        // TODO: remove this branch since it's not getting hit
         interfacesOutput += ' & ';
       }
 
-      interfacesOutput += '{' + EOL;
+      interfacesOutput += '{|' + EOL;
 
       for (const property of item.properties) {
         if (isAliasProperty(property)) {
           const generics = stringifyGenerics(property.generics, true);
-          interfacesOutput += `${JSON.stringify(property.name)}?: ${
-            item.fallback
-              ? `${property.alias.name + generics} | ${property.alias.name + generics}[],`
-              : property.alias.name + generics + ','
-          }`;
+          const key = JSON.stringify(property.name);
+          let type = property.alias.name + generics;
+          if (item.fallback) {
+            fallbackSet.add(type);
+            type = getNameForFallbackable(type);
+          }
+
+          interfacesOutput += `${key}?: ${type},`;
         } else {
           const value = stringifyTypes(property.type);
-          interfacesOutput += `${JSON.stringify(property.name)}?: ${
-            item.fallback ? `${value} | ${value}[],` : value + ','
-          }`;
+          const key = JSON.stringify(property.name);
+          let type = value;
+          if (item.fallback) {
+            fallbackSet.add(type);
+            type = getNameForFallbackable(type);
+          }
+
+          interfacesOutput += `${key}?: ${type},`;
         }
 
         interfacesOutput += EOL;
       }
 
-      interfacesOutput += '}';
+      interfacesOutput += '|}';
     }
   }
 
@@ -66,7 +95,24 @@ function flow() {
     ) + EOL}`;
   }
 
-  return `// @flow ${EOL + interfacesOutput + EOL + EOL + declarationsOutput + EOL}`;
+  const fallbacksOutput = [...fallbackSet]
+    .map(name => {
+      return `type ${getNameForFallbackable(name)} = ${name} | Array<${name}>;`;
+    })
+    .join(EOL);
+
+  return `// @flow strict ${EOL +
+    EOL +
+    '// See https://github.com/frenic/csstype/pull/67 for why all "fallbackable" types (e.g. `string | Array<string>`) are lifted here' +
+    EOL +
+    fallbacksOutput +
+    EOL +
+    EOL +
+    interfacesOutput +
+    EOL +
+    EOL +
+    declarationsOutput +
+    EOL}`;
 }
 
 function typescript() {

diff --git a/typecheck.js b/typecheck.js
@@ -1,11 +1,7 @@
-// @flow
-
+// @flow strict
 import * as CSS from './';
 
-// Fallback due to https://github.com/frenic/csstype/issues/17
-type Exact<T> = T & $Shape<T>;
-
-const css: Exact<CSS.Properties<*>> = {
+const css: CSS.Properties<*> = {
   flexGrow: 1,
   flexShrink: 1,
   flexBasis: '1px',
@@ -19,7 +15,7 @@ const css: Exact<CSS.Properties<*>> = {
   msOverflowStyle: 'scrollbar',
 };
 
-const cssWithFallbackValues: Exact<CSS.PropertiesFallback<*>> = {
+const cssWithFallbackValues: CSS.PropertiesFallback<*> = {
   flexGrow: [1],
   flexShrink: [1],
   flexBasis: ['1px'],
@@ -33,7 +29,7 @@ const cssWithFallbackValues: Exact<CSS.PropertiesFallback<*>> = {
   msOverflowStyle: ['scrollbar'],
 };
 
-const cssWithHyphenProperties: Exact<CSS.PropertiesHyphen<*>> = {
+const cssWithHyphenProperties: CSS.PropertiesHyphen<*> = {
   'flex-grow': 1,
   'flex-shrink': 0,
   'flex-basis': '1px',
@@ -47,11 +43,14 @@ const cssWithHyphenProperties: Exact<CSS.PropertiesHyphen<*>> = {
   '-ms-overflow-style': 'scrollbar',
 };
 
-const cssWithBothCamelAndHyphenProperties: $Exact<CSS.Properties<*>> & $Exact<CSS.PropertiesHyphen<*>> = {
+const cssWithBothCamelAndHyphenProperties: {|
+  ...CSS.Properties<*>,
+  ...CSS.PropertiesHyphen<*>,
+|} = {
   animation: '',
 };
 
-const atRuleFontFace: $Exact<CSS.FontFace> = {
+const atRuleFontFace: CSS.FontFace = {
   fontFamily: '',
   fontWeight: 'normal',
 };

diff --git a/yarn.lock b/yarn.lock
@@ -1146,10 +1146,10 @@ find-up@^3.0.0:
   dependencies:
     locate-path "^3.0.0"
 
-flow-bin@^0.94.0:
-  version "0.94.0"
-  resolved "https://registry.yarnpkg.com/flow-bin/-/flow-bin-0.94.0.tgz#b5d58fe7559705b73a18229f97edfc3ab6ffffcb"
-  integrity sha512-DYF7r9CJ/AksfmmB4+q+TyLMoeQPRnqtF1Pk7KY3zgfkB/nVuA3nXyzqgsIPIvnMSiFEXQcFK4z+iPxSLckZhQ==
+flow-bin@^0.98.0:
+  version "0.98.0"
+  resolved "https://registry.yarnpkg.com/flow-bin/-/flow-bin-0.98.0.tgz#3361a03682326a83a5f0a864749f4f7f0d826bce"
+  integrity sha512-vuiYjBVt82eYF+dEk9Zqa8hTSDvbhl/czxzFRLZm9/XHbJnYNMTwFoNFYAQT9IQ6ACNBIbwSTIfxroieuKja7g==
 
 for-in@^1.0.2:
   version "1.0.2"