Merge branch 'development' into refactor-remote-usage-in-app-theme-settings-to-main-process

app/src/lib/ipc-shared.ts

@@ -12,6 +12,7 @@ import { URLActionType } from './parse-app-url'
 import { Architecture } from './get-architecture'
 import { EndpointToken } from './endpoint-token'
 import { ThemeSource } from '../ui/lib/application-theme'
+import { PathType } from '../ui/lib/app-proxy'
 
 /**
  * Defines the simplex IPC channel names we use from the renderer
@@ -84,6 +85,7 @@ export type RequestChannels = {
  * Return signatures must be promises
  */
 export type RequestResponseChannels = {
+  'get-path': (path: PathType) => Promise<string>
   'get-app-architecture': () => Promise<Architecture>
   'get-app-path': () => Promise<string>
   'is-running-under-rosetta-translation': () => Promise<boolean>

app/src/lib/stores/app-store.ts

@@ -6185,7 +6185,7 @@ export class AppStore extends TypedBaseStore<IAppState> {
       await this.statsStore.recordTutorialStarted()
 
       const name = 'desktop-tutorial'
-      const path = Path.resolve(getDefaultDir(), name)
+      const path = Path.resolve(await getDefaultDir(), name)
 
       const apiRepository = await createTutorialRepository(
         account,

app/src/main-process/main.ts

@@ -549,6 +549,11 @@ app.on('ready', () => {
     }
   })
 
+  /**
+   * An event sent by the renderer asking for the app's architecture
+   */
+  ipcMain.handle('get-path', async (_, path) => app.getPath(path))
+
   /**
    * An event sent by the renderer asking for the app's architecture
    */

app/src/ui/add-repository/create-repository.tsx

@@ -52,7 +52,7 @@ interface ICreateRepositoryProps {
 }
 
 interface ICreateRepositoryState {
-  readonly path: string
+  readonly path: string | null
   readonly name: string
   readonly description: string
 
@@ -96,9 +96,7 @@ export class CreateRepository extends React.Component<
   public constructor(props: ICreateRepositoryProps) {
     super(props)
 
-    const path = this.props.initialPath
-      ? this.props.initialPath
-      : getDefaultDir()
+    const path = this.props.initialPath ? this.props.initialPath : null
 
     const name = this.props.initialPath
       ? sanitizedRepositoryName(Path.basename(this.props.initialPath))
@@ -118,6 +116,7 @@ export class CreateRepository extends React.Component<
       isRepository: false,
       readMeExists: false,
     }
+    this.initializePath()
   }
 
   public async componentDidMount() {
@@ -129,16 +128,23 @@ export class CreateRepository extends React.Component<
     const licenses = await getLicenses()
     this.setState({ licenses })
 
-    const isRepository = await isGitRepository(this.state.path)
+    const path =
+      this.state.path !== null ? this.state.path : await getDefaultDir()
+    const isRepository = await isGitRepository(path)
     this.setState({ isRepository })
 
-    this.updateReadMeExists(this.state.path, this.state.name)
+    this.updateReadMeExists(path, this.state.name)
   }
 
   public componentWillUnmount() {
     window.removeEventListener('focus', this.onWindowFocus)
   }
 
+  private initializePath = async () => {
+    const path = await getDefaultDir()
+    this.setState({ path })
+  }
+
   private onPathChanged = async (path: string) => {
     this.setState({ path, isValidPath: null })
 
@@ -175,8 +181,8 @@ export class CreateRepository extends React.Component<
     this.setState({ isRepository, path })
   }
 
-  private async updateReadMeExists(path: string, name: string) {
-    if (!enableReadmeOverwriteWarning()) {
+  private async updateReadMeExists(path: string | null, name: string) {
+    if (!enableReadmeOverwriteWarning() || path === null) {
       return
     }
 
@@ -187,8 +193,12 @@ export class CreateRepository extends React.Component<
     this.setState(state => (state.path === path ? { readMeExists } : null))
   }
 
-  private resolveRepositoryRoot = async (): Promise<string> => {
+  private resolveRepositoryRoot = async (): Promise<string | null> => {
     const currentPath = this.state.path
+    if (currentPath === null) {
+      return null
+    }
+
     if (this.props.initialPath && this.props.initialPath === currentPath) {
       // if the user provided an initial path and didn't change it, we should
       // validate it is an existing path and use that for the repository
@@ -204,6 +214,13 @@ export class CreateRepository extends React.Component<
   private createRepository = async () => {
     const fullPath = await this.resolveRepositoryRoot()
 
+    if (fullPath === null) {
+      // Shouldn't be able to get here with a null full path, but if you did,
+      // display error.
+      this.setState({ isValidPath: true })
+      return
+    }
+
     try {
       await FSE.ensureDir(fullPath)
       this.setState({ isValidPath: true })
@@ -350,7 +367,7 @@ export class CreateRepository extends React.Component<
     // don't update the default directory as a result of creating the
     // repository from an empty folder, because this value will be the
     // repository path itself
-    if (!this.props.initialPath) {
+    if (!this.props.initialPath && this.state.path !== null) {
       setDefaultDir(this.state.path)
     }
   }
@@ -500,6 +517,11 @@ export class CreateRepository extends React.Component<
   }
 
   private onAddRepositoryClicked = () => {
+    if (this.state.path === null) {
+      // Shouldn't be able to even get here if path is null.
+      return
+    }
+
     return this.props.dispatcher.showPopup({
       type: PopupType.AddRepository,
       path: this.state.path,
@@ -508,12 +530,14 @@ export class CreateRepository extends React.Component<
 
   public render() {
     const disabled =
+      this.state.path === null ||
       this.state.path.length === 0 ||
       this.state.name.length === 0 ||
       this.state.creating ||
       this.state.isRepository
 
     const readOnlyPath = !!this.props.initialPath
+    const loadingDefaultDir = this.state.path === null
 
     return (
       <Dialog
@@ -549,13 +573,16 @@ export class CreateRepository extends React.Component<
 
           <Row>
             <TextBox
-              value={this.state.path}
+              value={this.state.path ?? ''}
               label={__DARWIN__ ? 'Local Path' : 'Local path'}
               placeholder="repository path"
               onValueChanged={this.onPathChanged}
-              disabled={readOnlyPath}
+              disabled={readOnlyPath || loadingDefaultDir}
             />
-            <Button onClick={this.showFilePicker} disabled={readOnlyPath}>
+            <Button
+              onClick={this.showFilePicker}
+              disabled={readOnlyPath || loadingDefaultDir}
+            >
               Choose…
             </Button>
           </Row>
@@ -584,7 +611,7 @@ export class CreateRepository extends React.Component<
             okButtonText={
               __DARWIN__ ? 'Create Repository' : 'Create repository'
             }
-            okButtonDisabled={disabled}
+            okButtonDisabled={disabled || loadingDefaultDir}
           />
         </DialogFooter>
       </Dialog>

app/src/ui/clone-repository/clone-repository.tsx

@@ -77,7 +77,7 @@ interface ICloneRepositoryState {
    *
    *  See the onWindowFocus method for more information.
    */
-  readonly initialPath: string
+  readonly initialPath: string | null
 
   /** Are we currently trying to load the entered repository? */
   readonly loading: boolean
@@ -114,7 +114,7 @@ interface IBaseTabState {
   readonly lastParsedIdentifier: IRepositoryIdentifier | null
 
   /** The local path to clone to. */
-  readonly path: string
+  readonly path: string | null
 
   /** The user-entered URL or `owner/name` shortcut. */
   readonly url: string
@@ -151,7 +151,7 @@ export class CloneRepository extends React.Component<
   public constructor(props: ICloneRepositoryProps) {
     super(props)
 
-    const defaultDirectory = getDefaultDir()
+    const defaultDirectory = null
 
     const initialBaseTabState: IBaseTabState = {
       error: null,
@@ -180,6 +180,8 @@ export class CloneRepository extends React.Component<
         ...initialBaseTabState,
       },
     }
+
+    this.initializePath()
   }
 
   public componentDidUpdate(prevProps: ICloneRepositoryProps) {
@@ -197,6 +199,22 @@ export class CloneRepository extends React.Component<
     window.addEventListener('focus', this.onWindowFocus)
   }
 
+  private initializePath = async () => {
+    const initialPath = await getDefaultDir()
+    const dotComTabState = { ...this.state.dotComTabState, path: initialPath }
+    const enterpriseTabState = {
+      ...this.state.enterpriseTabState,
+      path: initialPath,
+    }
+    const urlTabState = { ...this.state.urlTabState, path: initialPath }
+    this.setState({
+      initialPath,
+      dotComTabState,
+      enterpriseTabState,
+      urlTabState,
+    })
+  }
+
   public componentWillUnmount() {
     window.removeEventListener('focus', this.onWindowFocus)
   }
@@ -235,7 +253,11 @@ export class CloneRepository extends React.Component<
     const { loading } = this.state
 
     const disabled =
-      url.length === 0 || path.length === 0 || loading || error !== null
+      url.length === 0 ||
+      path == null ||
+      path.length === 0 ||
+      loading ||
+      error !== null
 
     return disabled
   }
@@ -274,7 +296,7 @@ export class CloneRepository extends React.Component<
         const tabState = this.state.urlTabState
         return (
           <CloneGenericRepository
-            path={tabState.path}
+            path={tabState.path ?? ''}
             url={tabState.url}
             onPathChanged={this.onPathChanged}
             onUrlChanged={this.updateUrl}
@@ -297,7 +319,7 @@ export class CloneRepository extends React.Component<
 
           return (
             <CloneGithubRepository
-              path={tabState.path}
+              path={tabState.path ?? ''}
               account={account}
               selectedItem={tabState.selectedItem}
               onSelectionChanged={this.onSelectionChanged}
@@ -540,7 +562,7 @@ export class CloneRepository extends React.Component<
       buttonLabel: 'Select',
       nameFieldLabel: 'Clone As:',
       showsTagField: false,
-      defaultPath: tabState.path,
+      defaultPath: tabState.path ?? '',
       properties: ['createDirectory'],
     })
 
@@ -560,16 +582,17 @@ export class CloneRepository extends React.Component<
 
     let newPath: string
 
+    const dirPath = tabState.path ?? ''
     if (lastParsedIdentifier) {
       if (parsed) {
-        newPath = Path.join(Path.dirname(tabState.path), parsed.name)
+        newPath = Path.join(Path.dirname(dirPath), parsed.name)
       } else {
-        newPath = Path.dirname(tabState.path)
+        newPath = Path.dirname(dirPath)
       }
     } else if (parsed) {
-      newPath = Path.join(tabState.path, parsed.name)
+      newPath = Path.join(dirPath, parsed.name)
     } else {
-      newPath = tabState.path
+      newPath = dirPath
     }
 
     this.setSelectedTabState(
@@ -582,7 +605,15 @@ export class CloneRepository extends React.Component<
     )
   }
 
-  private async validateEmptyFolder(path: string): Promise<null | Error> {
+  private async validateEmptyFolder(
+    path: string | null
+  ): Promise<null | Error> {
+    if (path === null) {
+      return new Error(
+        'Unable to read path on disk. Please check the path and try again.'
+      )
+    }
+
     try {
       const directoryFiles = await readdir(path)
 
@@ -662,6 +693,13 @@ export class CloneRepository extends React.Component<
     const cloneInfo = await this.resolveCloneInfo()
     const { path } = this.getSelectedTabState()
 
+    if (path == null) {
+      const error = new Error(`Directory could not be created at this path.`)
+      this.setState({ loading: false })
+      this.setSelectedTabState({ error })
+      return
+    }
+
     if (!cloneInfo) {
       const error = new Error(
         `We couldn't find that repository. Check that you are logged in, the network is accessible, and the URL or repository alias are spelled correctly.`

app/src/ui/lib/app-proxy.ts

@@ -1,17 +1,26 @@
-import * as remote from '@electron/remote'
+import { getPath } from '../main-process-proxy'
 import { getAppPathProxy } from '../main-process-proxy'
 
-let app: Electron.App | null = null
 let path: string | null = null
 let documentsPath: string | null = null
 
-function getApp(): Electron.App {
-  if (!app) {
-    app = remote.app
-  }
-
-  return app
-}
+export type PathType =
+  | 'home'
+  | 'appData'
+  | 'userData'
+  | 'cache'
+  | 'temp'
+  | 'exe'
+  | 'module'
+  | 'desktop'
+  | 'documents'
+  | 'downloads'
+  | 'music'
+  | 'pictures'
+  | 'videos'
+  | 'recent'
+  | 'logs'
+  | 'crashDumps'
 
 /**
  * Get the version of the app.
@@ -47,14 +56,13 @@ export async function getAppPath(): Promise<string> {
  *
  * This is preferable to using `remote` directly because we cache the result.
  */
-export function getDocumentsPath(): string {
+export async function getDocumentsPath(): Promise<string> {
   if (!documentsPath) {
-    const app = getApp()
     try {
-      documentsPath = app.getPath('documents')
+      documentsPath = await getPath('documents')
     } catch (ex) {
       // a user profile may not have the Documents folder defined on Windows
-      documentsPath = app.getPath('home')
+      documentsPath = await getPath('home')
     }
   }
 

app/src/ui/lib/default-dir.ts

@@ -4,10 +4,10 @@ import { getDocumentsPath } from './app-proxy'
 const localStorageKey = 'last-clone-location'
 
 /** The path to the default directory. */
-export function getDefaultDir(): string {
+export async function getDefaultDir(): Promise<string> {
   return (
     localStorage.getItem(localStorageKey) ||
-    Path.join(getDocumentsPath(), 'GitHub')
+    Path.join(await getDocumentsPath(), 'GitHub')
   )
 }
 

app/src/ui/main-process-proxy.ts

@@ -178,6 +178,11 @@ export const showCertificateTrustDialog = sendProxy(
   2
 )
 
+/**
+ * Tell the main process to obtain the applications path for given path type
+ */
+export const getPath = invokeProxy('get-path', 1)
+
 /**
  * Tell the main process to obtain the applications architecture
  */