chore: Various typing fixes (#754)

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

amundsen_application/static/js/components/NavBar/index.spec.tsx

@@ -97,10 +97,12 @@ describe('NavBar', () => {
 
   describe('renderSearchBar', () => {
     it('returns small SearchBar when not on home page', () => {
-      const { props, wrapper } = setup(null, { pathname: '/search' });
-      const searchBar = shallow(wrapper.instance().renderSearchBar()).find(
-        SearchBar
-      );
+      const { props, wrapper } = setup(undefined, { pathname: '/search' });
+      const rendered = wrapper.instance().renderSearchBar();
+      if (rendered === null) {
+        throw Error('rendering search bar returned null');
+      }
+      const searchBar = shallow(rendered).find(SearchBar);
       expect(searchBar.exists()).toBe(true);
       expect(searchBar.props()).toMatchObject({
         size: 'small',
@@ -108,7 +110,7 @@ describe('NavBar', () => {
     });
 
     it('returns null if conditions to render search bar are not met', () => {
-      const { props, wrapper } = setup(null, { pathname: '/' });
+      const { props, wrapper } = setup(undefined, { pathname: '/' });
       expect(wrapper.instance().renderSearchBar()).toBe(null);
     });
   });

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

@@ -19,10 +19,10 @@ describe('OwnerEditor', () => {
       errorText: null,
       isLoading: false,
       itemProps: {},
-      isEditing: null,
+      isEditing: undefined,
       setEditMode: jest.fn(),
       onUpdateList: jest.fn(),
-      readOnly: null,
+      readOnly: undefined,
       resourceType: ResourceType.table,
       ...propOverrides,
     };

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

@@ -145,9 +145,10 @@ export class OwnerEditor extends React.Component<
 
   recordAddItem = (event: React.FormEvent<HTMLFormElement>) => {
     event.preventDefault();
+    if (this.inputRef.current) {
       const { value } = this.inputRef.current;
 
-    if (this.inputRef.current && value) {
+      if (value) {
         this.inputRef.current.value = '';
 
         const newTempItemProps = {
@@ -156,6 +157,7 @@ export class OwnerEditor extends React.Component<
         };
         this.setState({ tempItemProps: newTempItemProps });
       }
+    }
   };
 
   recordDeleteItem = (deletedKey: string) => {

amundsen_application/static/js/components/common/SearchBar/InlineSearchResults/SearchItemList/SearchItem/tests/index.spec.tsx

@@ -60,8 +60,11 @@ describe('SearchItem', () => {
 
     it('renders correct text if !props.hasResults', () => {
       const { wrapper } = setup({ hasResults: false });
-      const content = shallow(wrapper.instance().renderIndicator());
-      expect(content.text()).toBe(SEARCH_ITEM_NO_RESULTS);
+      const rendered = wrapper.instance().renderIndicator();
+      if (rendered === null) {
+        throw Error('renderIndicator returned null');
+      }
+      expect(shallow(rendered).text()).toBe(SEARCH_ITEM_NO_RESULTS);
     });
 
     it('renders nothing if !props.Loading and props.hasResults', () => {

amundsen_application/static/js/ducks/bookmark/reducer.ts

@@ -121,12 +121,17 @@ export default function reducer(
 ): BookmarkReducerState {
   switch (action.type) {
     case AddBookmark.SUCCESS:
-    case GetBookmarks.SUCCESS:
+    case GetBookmarks.SUCCESS: {
+      const { payload } = <GetBookmarksResponse>action;
+      if (payload === undefined) {
+        throw Error('payload must be set for GetBookmarks.SUCCESS');
+      }
       return {
         ...state,
-        myBookmarks: (<GetBookmarksResponse>action).payload.bookmarks,
+        myBookmarks: payload.bookmarks,
         myBookmarksIsLoaded: true,
       };
+    }
     case GetBookmarksForUser.REQUEST:
       return {
         ...state,
@@ -134,16 +139,23 @@ export default function reducer(
           ...initialBookmarkState,
         },
       };
-    case GetBookmarksForUser.SUCCESS:
+    case GetBookmarksForUser.SUCCESS: {
+      const { payload } = <GetBookmarksForUserResponse>action;
+
+      if (payload === undefined) {
+        throw Error('payload must be set for GetBookmarksForUser.SUCCESS');
+      }
       return {
         ...state,
-        bookmarksForUser: (<GetBookmarksForUserResponse>action).payload
-          .bookmarks,
+        bookmarksForUser: payload.bookmarks,
       };
-    case RemoveBookmark.SUCCESS:
-      const { resourceKey, resourceType } = (<RemoveBookmarkResponse>(
-        action
-      )).payload;
+    }
+    case RemoveBookmark.SUCCESS: {
+      const { payload } = <RemoveBookmarkResponse>action;
+      if (payload === undefined) {
+        throw Error('payload must be set for RemoveBookmark.SUCCESS');
+      }
+      const { resourceKey, resourceType } = payload;
       return {
         ...state,
         myBookmarks: {
@@ -153,6 +165,7 @@ export default function reducer(
           ),
         },
       };
+    }
     case AddBookmark.FAILURE:
     case GetBookmarks.FAILURE:
     case GetBookmarksForUser.FAILURE:

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

@@ -61,8 +61,8 @@ export interface SearchReducerState {
 export function searchAll(
   searchType: SearchType,
   term: string,
-  resource?: ResourceType,
-  pageIndex?: number,
+  resource: ResourceType,
+  pageIndex: number,
   useFilters: boolean = false
 ): SearchAllRequest {
   return {
@@ -314,6 +314,11 @@ export default function reducer(
     case SearchAll.SUCCESS:
       // resets all resources with initial state then applies search results
       const newState = (<SearchAllResponse>action).payload;
+      if (newState === undefined) {
+        throw Error(
+          'SearchAllResponse.payload must be specified for SUCCESS type'
+        );
+      }
       return {
         ...initialState,
         ...newState,
@@ -354,6 +359,11 @@ export default function reducer(
       };
     case InlineSearch.SUCCESS:
       const inlineResults = (<InlineSearchResponse>action).payload;
+      if (inlineResults === undefined) {
+        throw Error(
+          'InlineSearchResponse.payload must be specified for SUCCESS type'
+        );
+      }
       return {
         ...state,
         inlineResults: {

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

@@ -112,6 +112,7 @@ export function* submitSearchResourceWatcher(): SagaIterator {
 export function* updateSearchStateWorker(
   action: UpdateSearchStateRequest
 ): SagaIterator {
+  if (action.payload !== undefined) {
     const { filters, resource, updateUrl, submitSearch } = action.payload;
     const state = yield select(getSearchState);
     if (filters && submitSearch) {
@@ -124,6 +125,7 @@ export function* updateSearchStateWorker(
         filters: filters || state.filters,
       });
     }
+  }
 }
 export function* updateSearchStateWatcher(): SagaIterator {
   yield takeEvery(UpdateSearchState.REQUEST, updateSearchStateWorker);

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

@@ -20,11 +20,11 @@ export const getPageIndex = (
   resource = resource || state.resource;
   switch (resource) {
     case ResourceType.table:
-      return state.tables.page_index;
+      return state.tables?.page_index || 0;
     case ResourceType.user:
-      return state.users.page_index;
+      return state.users?.page_index || 0;
     case ResourceType.dashboard:
-      return state.dashboards.page_index;
+      return state.dashboards?.page_index || 0;
   }
   return 0;
 };

amundsen_application/static/js/ducks/tableMetadata/api/helpers.spec.ts

@@ -180,7 +180,7 @@ describe('helpers', () => {
     it('returns false if not a user with display_name', () => {
       const testUser = {
         ...globalState.user.loggedInUser,
-        display_name: null,
+        display_name: '',
       };
       expect(Helpers.shouldSendNotification(testUser)).toBe(false);
     });

amundsen_application/static/js/ducks/tableMetadata/index.spec.ts

@@ -483,7 +483,7 @@ describe('tableMetadata ducks', () => {
           sagaTest = (mockSuccess) => {
             return testSaga(
               updateTableDescriptionWorker,
-              updateTableDescription(newDescription, mockSuccess, null)
+              updateTableDescription(newDescription, mockSuccess, undefined)
             )
               .next()
               .select()
@@ -510,7 +510,7 @@ describe('tableMetadata ducks', () => {
           sagaTest = (mockFailure) => {
             return testSaga(
               updateTableDescriptionWorker,
-              updateTableDescription(newDescription, null, mockFailure)
+              updateTableDescription(newDescription, undefined, mockFailure)
             )
               .next()
               .select()
@@ -623,7 +623,7 @@ describe('tableMetadata ducks', () => {
                 newDescription,
                 columnIndex,
                 mockSuccess,
-                null
+                undefined
               )
             )
               .next()
@@ -655,7 +655,7 @@ describe('tableMetadata ducks', () => {
               updateColumnDescription(
                 newDescription,
                 columnIndex,
-                null,
+                undefined,
                 mockFailure
               )
             )

amundsen_application/static/js/fixtures/globalState.ts

@@ -67,7 +67,7 @@ const globalState: GlobalState = {
   },
   issue: {
     issues: [],
-    allIssuesUrl: null,
+    allIssuesUrl: '',
     total: 0,
     isLoading: true,
   },

amundsen_application/static/js/fixtures/metadata/users.ts

-export const activeUser0 = {
-  manager_id: null,
-  manager_fullname: null,
-  manager_email: null,
+import { PeopleUser } from 'interfaces/User';
+
+export const activeUser0: PeopleUser = {
+  manager_fullname: undefined,
+  manager_email: undefined,
   profile_url: '/test0',
-  role_name: null,
-  display_name: null,
-  github_username: null,
-  team_name: null,
-  last_name: null,
-  full_name: null,
-  slack_id: null,
-  first_name: null,
-  employee_type: null,
-  other_key_values: {},
+  role_name: undefined,
+  display_name: '',
+  github_username: '',
+  team_name: '',
+  last_name: '',
+  full_name: '',
+  slack_id: '',
+  first_name: '',
+  employee_type: '',
   is_active: true,
   email: 'user0@test.com',
   user_id: 'user0',
 };
 
-export const activeUser1 = {
-  manager_id: null,
-  manager_fullname: null,
-  manager_email: null,
+export const activeUser1: PeopleUser = {
+  manager_fullname: undefined,
+  manager_email: undefined,
   profile_url: '/test1',
-  role_name: null,
-  display_name: null,
-  github_username: null,
-  team_name: null,
-  last_name: null,
-  full_name: null,
-  slack_id: null,
-  first_name: null,
-  employee_type: null,
-  other_key_values: {},
+  role_name: undefined,
+  display_name: '',
+  github_username: '',
+  team_name: '',
+  last_name: '',
+  full_name: '',
+  slack_id: '',
+  first_name: '',
+  employee_type: '',
   is_active: true,
   email: 'user10@test.com',
   user_id: 'user1',

amundsen_application/static/js/fixtures/mockRouter.ts

@@ -5,7 +5,7 @@ import * as History from 'history';
 // Mock React-Router
 export function getMockRouterProps<P>(
   data: P,
-  location: Partial<History.Location>
+  location: Partial<History.Location> = {}
 ): RouteComponentProps<P> {
   const mockLocation: History.Location = {
     hash: '',
@@ -24,7 +24,10 @@ export function getMockRouterProps<P>(
       url: '',
     },
     location: mockLocation,
-    history: {
+    // This history object is a mock and `null`s many of the required methods. The
+    // tests are designed not to trigger them, if they do, an error is expected.
+    // So cast this as any.
+    history: <any>{
       length: 2,
       action: 'POP',
       location: mockLocation,

amundsen_application/static/js/fixtures/search/inlineResults.ts

@@ -99,7 +99,7 @@ export const allResourcesExample = {
       {
         display_name: 'Test User',
         email: 'tuser@test.com',
-        employee_type: null,
+        employee_type: '',
         first_name: 'Test',
         full_name: 'Test User',
         github_username: '',
@@ -108,8 +108,8 @@ export const allResourcesExample = {
         manager_email: 'tuser2@test.com',
         manager_fullname: 'Test User2',
         profile_url: '',
-        role_name: null,
-        slack_id: null,
+        role_name: undefined,
+        slack_id: '',
         team_name: 'Amundsen Team',
         type: 'user',
         user_id: 'tuser@test.com',
@@ -117,7 +117,7 @@ export const allResourcesExample = {
       {
         display_name: 'Test User2',
         email: 'tuser2@test.com',
-        employee_type: null,
+        employee_type: '',
         first_name: 'Test',
         full_name: 'Test User2',
         github_username: '',
@@ -126,8 +126,8 @@ export const allResourcesExample = {
         manager_email: 'tuser3@test.com',
         manager_fullname: 'Test User3',
         profile_url: '',
-        role_name: null,
-        slack_id: null,
+        role_name: undefined,
+        slack_id: '',
         team_name: 'Amundsen Team',
         type: 'user',
         user_id: 'tuser2@test.com',

amundsen_application/static/js/pages/SearchPage/index.spec.tsx

@@ -67,7 +67,7 @@ describe('SearchPage', () => {
     let wrapper;
 
     beforeAll(() => {
-      ({ props, wrapper } = setup(null, {
+      ({ props, wrapper } = setup(undefined, {
         search: '/search?searchTerm=testName&resource=table&pageIndex=1',
       }));
     });
@@ -84,7 +84,7 @@ describe('SearchPage', () => {
     let mockPrevProps;
 
     beforeAll(() => {
-      ({ props, wrapper } = setup(null, {
+      ({ props, wrapper } = setup(undefined, {
         search: '/search?searchTerm=testName&resource=table&pageIndex=1',
       }));
       mockPrevProps = {
@@ -267,7 +267,11 @@ describe('SearchPage', () => {
         resource: ResourceType.table,
       });
       const getTabContentSpy = jest.spyOn(wrapper.instance(), 'getTabContent');
-      shallow(wrapper.instance().renderSearchResults());
+      const rendered = wrapper.instance().renderSearchResults();
+      if (rendered === null) {
+        throw Error('renderSearchResults returned null');
+      }
+      shallow(rendered);
       expect(getTabContentSpy).toHaveBeenCalledWith(
         props.tables,
         ResourceType.table
@@ -279,7 +283,11 @@ describe('SearchPage', () => {
         resource: ResourceType.user,
       });
       const getTabContentSpy = jest.spyOn(wrapper.instance(), 'getTabContent');
-      shallow(wrapper.instance().renderSearchResults());
+      const rendered = wrapper.instance().renderSearchResults();
+      if (rendered === null) {
+        throw Error('renderSearchResults returned null');
+      }
+      shallow(rendered);
       expect(getTabContentSpy).toHaveBeenCalledWith(
         props.users,
         ResourceType.user
@@ -291,7 +299,11 @@ describe('SearchPage', () => {
         resource: ResourceType.dashboard,
       });
       const getTabContentSpy = jest.spyOn(wrapper.instance(), 'getTabContent');
-      shallow(wrapper.instance().renderSearchResults());
+      const rendered = wrapper.instance().renderSearchResults();
+      if (rendered === null) {
+        throw Error('renderSearchResults returned null');
+      }
+      shallow(rendered);
       expect(getTabContentSpy).toHaveBeenCalledWith(
         props.dashboards,
         ResourceType.dashboard
@@ -300,7 +312,7 @@ describe('SearchPage', () => {
 
     it('renders null for an invalid resource', () => {
       const { wrapper } = setup({
-        resource: null,
+        resource: undefined,
       });
       const renderedSearchResults = wrapper.instance().renderSearchResults();
       expect(renderedSearchResults).toBe(null);

amundsen_application/static/js/utils/textUtils.ts

@@ -8,7 +8,7 @@ export enum CaseType {
   TITLE_CASE = 'titleCase',
 }
 
-export function convertText(str = '', caseType: CaseType): string {
+export function convertText(str = '', caseType: CaseType | null): string {
   switch (caseType) {
     case CaseType.LOWER_CASE:
       return str.toLowerCase();

amundsen_application/static/webpack.common.ts

@@ -36,7 +36,7 @@ const PATHS = {
 };
 
 // Process of Templates
-const walkSync = (dir, filelist = []) => {
+const walkSync = (dir: string, filelist: string[] = []) => {
   fs.readdirSync(dir).forEach((file) => {
     filelist = fs.statSync(path.join(dir, file)).isDirectory()
       ? walkSync(path.join(dir, file), filelist)