build: Adding betterer to workflow (#598)

Signed-off-by: Marcos Iglesias Valle <golodhros@gmail.com>

.github/workflows/pull_request.yml

@@ -15,8 +15,8 @@ jobs:
   test-unit-python:
     runs-on: ubuntu-18.04
     strategy:
-        matrix:
-          python-version: ['3.6.x', '3.7.x']
+      matrix:
+        python-version: ["3.6.x", "3.7.x"]
     steps:
       - name: Checkout
         uses: actions/checkout@v1
@@ -34,20 +34,24 @@ jobs:
       matrix:
         node-version: [12.x]
     steps:
-    - uses: actions/checkout@v2
-    - name: Use Node.js ${{ matrix.node-version }}
-      uses: actions/setup-node@v1
-      with:
-        node-version: ${{ matrix.node-version }}
-    - name: Install and Lint
-      working-directory: ./amundsen_application/static
-      run: |
-        npm install
-        npm install codecov -g
-        npm run lint
-        npm run build --if-present
-    - name: Test
-      working-directory: ./amundsen_application/static
-      run: npm run test
-      env:
-        CI: true
+      - uses: actions/checkout@v2
+      - name: Use Node.js ${{ matrix.node-version }}
+        uses: actions/setup-node@v1
+        with:
+          node-version: ${{ matrix.node-version }}
+      - name: Install and Lint
+        working-directory: ./amundsen_application/static
+        run: |
+          npm install
+          npm install codecov -g
+          npm run lint
+      - name: Check and build
+        working-directory: ./amundsen_application/static
+        run: |
+          npm run betterer
+          npm run build --if-present
+      - name: Test
+        working-directory: ./amundsen_application/static
+        run: npm run test
+        env:
+          CI: true

amundsen_application/static/.betterer.ts

+import { typescript } from '@betterer/typescript';
+
+export default {
+  'strict null compilation': typescript('./tsconfig.json', {
+    strictNullChecks: true,
+  }),
+};

amundsen_application/static/js/components/DashboardPage/index.tsx

@@ -52,6 +52,12 @@ interface DashboardPageState {
   uri: string;
 }
 
+type TabInfo = {
+  content: JSX.Element;
+  key: string;
+  title: string;
+};
+
 export interface StateFromProps {
   isLoading: boolean;
   statusCode: number;
@@ -111,7 +117,8 @@ export class DashboardPage extends React.Component<
   };
 
   renderTabs() {
-    const tabInfo = [];
+    const tabInfo: TabInfo[] = [];
+
     tabInfo.push({
       content: (
         <ResourceList

amundsen_application/static/js/components/common/Flag/index.spec.tsx

@@ -60,8 +60,8 @@ describe('Flag', () => {
       expect(convertText(text, 'not a valid options')).toEqual(text);
     });
 
-    it('returns empty strings for null values', () => {
-      expect(convertText(null, CaseType.SENTENCE_CASE)).toEqual('');
+    it('returns empty strings for undefined values', () => {
+      expect(convertText(undefined, CaseType.SENTENCE_CASE)).toEqual('');
     });
   });
 });

amundsen_application/static/js/components/common/Flag/index.tsx

@@ -19,8 +19,7 @@ export interface FlagProps {
   labelStyle?: BadgeStyle;
 }
 
-export function convertText(str: string, caseType: string): string {
-  str = str || '';
+export function convertText(str = '', caseType: string): string {
   switch (caseType) {
     case CaseType.LOWER_CASE:
       return str.toLowerCase();
@@ -34,9 +33,9 @@ export function convertText(str: string, caseType: string): string {
 }
 
 const Flag: React.FC<FlagProps> = ({
-  caseType,
-  text,
-  labelStyle,
+  caseType = null,
+  text = '',
+  labelStyle = BadgeStyle.DEFAULT,
 }: FlagProps) => {
   // TODO: After upgrading to Bootstrap 4, this component should leverage badges
   // https://getbootstrap.com/docs/4.1/components/badge/
@@ -49,10 +48,4 @@ const Flag: React.FC<FlagProps> = ({
   );
 };
 
-Flag.defaultProps = {
-  caseType: null,
-  text: '',
-  labelStyle: BadgeStyle.DEFAULT,
-};
-
 export default Flag;

amundsen_application/static/js/ducks/search/utils.ts

@@ -10,7 +10,7 @@ TODO: Coupling the shape of the search state and search response requires the us
 Partial to resolve errors, removing type safty of these methods. We should
 restructure any logic that takes of advantage of the case where the shape of the response
 and the shape of the state happend to be the same because a piece of application state
-can be the combination of multiple responses. 
+can be the combination of multiple responses.
 */
 
 export const getPageIndex = (

amundsen_application/static/js/fixtures/globalState.ts

@@ -154,7 +154,7 @@ const globalState: GlobalState = {
       data: {},
       status: null,
     },
-    statusCode: null,
+    statusCode: 200,
     tableData: {
       badges: [],
       cluster: '',

amundsen_application/static/package.json

@@ -26,7 +26,9 @@
     "stylelint:fix": "stylelint --fix '**/*.scss'",
     "format": "prettier --loglevel warn --write \"**/*.{ts,tsx,css,scss}\"",
     "storybook": "cross-env TS_NODE_PROJECT='tsconfig.webpack.json' start-storybook -p 6006",
-    "build-storybook": "cross-env TS_NODE_PROJECT='tsconfig.webpack.json' build-storybook"
+    "build-storybook": "cross-env TS_NODE_PROJECT='tsconfig.webpack.json' build-storybook",
+    "betterer": "betterer",
+    "betterer:update": "betterer --update"
   },
   "author": "",
   "license": "Apache-2.0",
@@ -57,6 +59,8 @@
     "@storybook/addons": "^5.3.19",
     "@storybook/react": "^5.3.19",
     "@storybook/theming": "^6.0.6",
+    "@betterer/cli": "^3.0.0",
+    "@betterer/typescript": "^3.0.1",
     "@types/enzyme": "^3.10.5",
     "@types/jasmine-matchers": "^0.2.32",
     "@types/jest": "^24.9.1",

docs/developer_guide.md

@@ -60,6 +60,10 @@ Using Storybook makes it much easier to quickly iterate on components when getti
 
 We use TypeScript in our codebase, so `npm run tsc` conducts type checking. The build commands `npm run build` and `npm run dev-build` also conduct type checking, but are slower because they also build the source code. Run any of these commands and fix all failed checks before submitting a PR.
 
+Currently, we are trying to gradually make our TypeScript code more strict. For that, we are leveraging a project called [betterer][betterer], which keeps track of our errors when a given test is passed. Right now, we are running it with "strictNullChecks" set to true, so if any code change makes the results worse, it will break the build.
+
+[betterer]: https://github.com/phenomnomnominal/betterer
+
 ### Frontend Linting and Formatting
 
 We have in place two linters – [ESLint][eslint] for our JavaScript and TypeScript files, [Stylelint][stylelint] for our Sass files. If you have both ESLint and Stylelint extensions installed on your IDE, you should get warnings on your editor by default.