From c56c70cf7b80b42694be66c29e7d0b28e538ac56 Mon Sep 17 00:00:00 2001
From: blam <ben@blam.sh>
Date: Tue, 18 Jan 2022 08:16:51 +0100
Subject: [PATCH] chore: fixing up code review comments for the RepoUrlPicker

Signed-off-by: blam <ben@blam.sh>
---
 .../fields/RepoUrlPicker/AzureRepoPicker.tsx  |  7 +---
 .../RepoUrlPicker/BitbucketRepoPicker.tsx     |  7 +---
 .../fields/RepoUrlPicker/GithubRepoPicker.tsx |  8 +---
 .../fields/RepoUrlPicker/GitlabRepoPicker.tsx |  8 +---
 .../fields/RepoUrlPicker/RepoUrlPicker.tsx    | 39 ++++++++++---------
 .../RepoUrlPicker/RepoUrlPickerHost.tsx       |  8 +---
 6 files changed, 31 insertions(+), 46 deletions(-)

diff --git a/plugins/scaffolder/src/components/fields/RepoUrlPicker/AzureRepoPicker.tsx b/plugins/scaffolder/src/components/fields/RepoUrlPicker/AzureRepoPicker.tsx
index 584cce7eed..0efadf4017 100644
--- a/plugins/scaffolder/src/components/fields/RepoUrlPicker/AzureRepoPicker.tsx
+++ b/plugins/scaffolder/src/components/fields/RepoUrlPicker/AzureRepoPicker.tsx
@@ -21,15 +21,12 @@ import Input from '@material-ui/core/Input';
 import InputLabel from '@material-ui/core/InputLabel';
 import { RepoUrlPickerState } from './types';
 
-export const AzureRepoPicker = ({
-  rawErrors,
-  state,
-  onChange,
-}: {
+export const AzureRepoPicker = (props: {
   state: RepoUrlPickerState;
   onChange: (state: RepoUrlPickerState) => void;
   rawErrors: string[];
 }) => {
+  const { rawErrors, state, onChange } = props;
   const { organization, repoName, owner } = state;
   return (
     <>
diff --git a/plugins/scaffolder/src/components/fields/RepoUrlPicker/BitbucketRepoPicker.tsx b/plugins/scaffolder/src/components/fields/RepoUrlPicker/BitbucketRepoPicker.tsx
index 5b4f271d74..64eb4b8dac 100644
--- a/plugins/scaffolder/src/components/fields/RepoUrlPicker/BitbucketRepoPicker.tsx
+++ b/plugins/scaffolder/src/components/fields/RepoUrlPicker/BitbucketRepoPicker.tsx
@@ -20,15 +20,12 @@ import Input from '@material-ui/core/Input';
 import InputLabel from '@material-ui/core/InputLabel';
 import { RepoUrlPickerState } from './types';
 
-export const BitbucketRepoPicker = ({
-  onChange,
-  rawErrors,
-  state,
-}: {
+export const BitbucketRepoPicker = (props: {
   onChange: (state: RepoUrlPickerState) => void;
   state: RepoUrlPickerState;
   rawErrors: string[];
 }) => {
+  const { onChange, rawErrors, state } = props;
   const { host, workspace, project, repoName } = state;
   return (
     <>
diff --git a/plugins/scaffolder/src/components/fields/RepoUrlPicker/GithubRepoPicker.tsx b/plugins/scaffolder/src/components/fields/RepoUrlPicker/GithubRepoPicker.tsx
index b59d089a19..13b57cb449 100644
--- a/plugins/scaffolder/src/components/fields/RepoUrlPicker/GithubRepoPicker.tsx
+++ b/plugins/scaffolder/src/components/fields/RepoUrlPicker/GithubRepoPicker.tsx
@@ -21,17 +21,13 @@ import InputLabel from '@material-ui/core/InputLabel';
 import { Select, SelectItem } from '@backstage/core-components';
 import { RepoUrlPickerState } from './types';
 
-export const GithubRepoPicker = ({
-  allowedOwners = [],
-  rawErrors,
-  state,
-  onChange,
-}: {
+export const GithubRepoPicker = (props: {
   allowedOwners?: string[];
   rawErrors: string[];
   state: RepoUrlPickerState;
   onChange: (state: RepoUrlPickerState) => void;
 }) => {
+  const { allowedOwners = [], rawErrors, state, onChange } = props;
   const ownerItems: SelectItem[] = allowedOwners
     ? allowedOwners.map(i => ({ label: i, value: i }))
     : [{ label: 'Loading...', value: 'loading' }];
diff --git a/plugins/scaffolder/src/components/fields/RepoUrlPicker/GitlabRepoPicker.tsx b/plugins/scaffolder/src/components/fields/RepoUrlPicker/GitlabRepoPicker.tsx
index 0e4e7dcd8f..1c63852866 100644
--- a/plugins/scaffolder/src/components/fields/RepoUrlPicker/GitlabRepoPicker.tsx
+++ b/plugins/scaffolder/src/components/fields/RepoUrlPicker/GitlabRepoPicker.tsx
@@ -21,17 +21,13 @@ import InputLabel from '@material-ui/core/InputLabel';
 import { Select, SelectItem } from '@backstage/core-components';
 import { RepoUrlPickerState } from './types';
 
-export const GitlabRepoPicker = ({
-  allowedOwners = [],
-  rawErrors,
-  state,
-  onChange,
-}: {
+export const GitlabRepoPicker = (props: {
   allowedOwners?: string[];
   state: RepoUrlPickerState;
   onChange: (state: RepoUrlPickerState) => void;
   rawErrors: string[];
 }) => {
+  const { allowedOwners = [], rawErrors, state, onChange } = props;
   const ownerItems: SelectItem[] = allowedOwners
     ? allowedOwners.map(i => ({ label: i, value: i }))
     : [{ label: 'Loading...', value: 'loading' }];
diff --git a/plugins/scaffolder/src/components/fields/RepoUrlPicker/RepoUrlPicker.tsx b/plugins/scaffolder/src/components/fields/RepoUrlPicker/RepoUrlPicker.tsx
index 65b1f8bf61..998fa41588 100644
--- a/plugins/scaffolder/src/components/fields/RepoUrlPicker/RepoUrlPicker.tsx
+++ b/plugins/scaffolder/src/components/fields/RepoUrlPicker/RepoUrlPicker.tsx
@@ -30,18 +30,19 @@ export interface RepoUrlPickerUiOptions {
   allowedOwners?: string[];
 }
 
-export const RepoUrlPicker = ({
-  uiSchema,
-  onChange,
-  rawErrors,
-  formData,
-}: FieldExtensionComponentProps<string, RepoUrlPickerUiOptions>) => {
+export const RepoUrlPicker = (
+  props: FieldExtensionComponentProps<string, RepoUrlPickerUiOptions>,
+) => {
+  const { uiSchema, onChange, rawErrors, formData } = props;
   const [state, setState] = useState<RepoUrlPickerState>(
     parseRepoPickerUrl(formData),
   );
   const integrationApi = useApi(scmIntegrationsApiRef);
 
-  const allowedHosts = uiSchema?.['ui:options']?.allowedHosts ?? [];
+  const allowedHosts = useMemo(
+    () => uiSchema?.['ui:options']?.allowedHosts ?? [],
+    [uiSchema],
+  );
   const allowedOwners = useMemo(
     () => uiSchema?.['ui:options']?.allowedOwners ?? [],
     [uiSchema],
@@ -65,6 +66,9 @@ export const RepoUrlPicker = ({
     [setState],
   );
 
+  const hostType =
+    (state.host && integrationApi.byHost(state.host)?.type) ?? null;
+
   return (
     <>
       <RepoUrlPickerHost
@@ -73,7 +77,7 @@ export const RepoUrlPicker = ({
         onChange={host => setState(prevState => ({ ...prevState, host }))}
         rawErrors={rawErrors}
       />
-      {state.host && integrationApi.byHost(state.host)?.type === 'github' && (
+      {hostType === 'github' && (
         <GithubRepoPicker
           allowedOwners={allowedOwners}
           rawErrors={rawErrors}
@@ -81,7 +85,7 @@ export const RepoUrlPicker = ({
           onChange={updateLocalState}
         />
       )}
-      {state.host && integrationApi.byHost(state.host)?.type === 'gitlab' && (
+      {hostType === 'gitlab' && (
         <GitlabRepoPicker
           allowedOwners={allowedOwners}
           rawErrors={rawErrors}
@@ -89,15 +93,14 @@ export const RepoUrlPicker = ({
           onChange={updateLocalState}
         />
       )}
-      {state.host &&
-        integrationApi.byHost(state.host)?.type === 'bitbucket' && (
-          <BitbucketRepoPicker
-            rawErrors={rawErrors}
-            state={state}
-            onChange={updateLocalState}
-          />
-        )}
-      {state.host && integrationApi.byHost(state.host)?.type === 'azure' && (
+      {hostType === 'bitbucket' && (
+        <BitbucketRepoPicker
+          rawErrors={rawErrors}
+          state={state}
+          onChange={updateLocalState}
+        />
+      )}
+      {hostType === 'azure' && (
         <AzureRepoPicker
           rawErrors={rawErrors}
           state={state}
diff --git a/plugins/scaffolder/src/components/fields/RepoUrlPicker/RepoUrlPickerHost.tsx b/plugins/scaffolder/src/components/fields/RepoUrlPicker/RepoUrlPickerHost.tsx
index 15777fde81..261429c757 100644
--- a/plugins/scaffolder/src/components/fields/RepoUrlPicker/RepoUrlPickerHost.tsx
+++ b/plugins/scaffolder/src/components/fields/RepoUrlPicker/RepoUrlPickerHost.tsx
@@ -21,17 +21,13 @@ import { useApi } from '@backstage/core-plugin-api';
 import { scaffolderApiRef } from '../../../api';
 import useAsync from 'react-use/lib/useAsync';
 
-export const RepoUrlPickerHost = ({
-  host,
-  hosts,
-  onChange,
-  rawErrors,
-}: {
+export const RepoUrlPickerHost = (props: {
   host?: string;
   hosts?: string[];
   onChange: (host: string) => void;
   rawErrors: string[];
 }) => {
+  const { host, hosts, onChange, rawErrors } = props;
   const scaffolderApi = useApi(scaffolderApiRef);
 
   const { value: integrations, loading } = useAsync(async () => {