development/github-desktop
development/github-desktop
1
0
Fork 0
mirror of https://github.com/desktop/desktop synced 2024-10-02 14:23:59 +00:00
Code Issues Packages Projects Releases Wiki Activity

Revert setAlmostImmediate

Browse source
This commit is contained in:
Sergio Padrino 2021-11-17 13:58:59 +01:00
parent b117361703
commit 81297bceb0
11 changed files with 13 additions and 258 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

1
.eslintrc.yml
View file

@ -25,7 +25,6 @@ rules:
react-no-unbound-dispatcher-props: error
react-readonly-props-and-state: error
react-proper-lifecycle-methods: error
set-almost-immediate: error
###########
# PLUGINS #

5
app/src/lib/feature-flag.ts
View file

@ -143,11 +143,6 @@ export function enableSSHAskPass(): boolean {
return true
}
/** Should we use the setImmediate alternative? */
export function enableSetAlmostImmediate(): boolean {
return false
}
/** Should we show ci check runs? */
export function enableCICheckRuns(): boolean {
return enableBetaFeatures()

35
app/src/lib/set-almost-immediate.ts
View file

@ -1,35 +0,0 @@
/* eslint-disable set-almost-immediate */
import { enableSetAlmostImmediate } from './feature-flag'
/**
* Reference created by setAlmostImmediate so that it can be cleared later.
* It can be NodeJS.Immediate or number because we use a feature flag
* to tweak the behavior of setAlmostImmediate, but this type should be used
* as if it were opaque.
*/
export type AlmostImmediate = NodeJS.Immediate | number
/**
* This function behaves almost like setImmediate, but it will rely on
* setTimeout(..., 0) to actually execute the callback. The reason for this
* is a bug in Electron sometimes causing setImmediate callbacks to not being
* executed.
* For more info about this: https://github.com/electron/electron/issues/29261
*/
export function setAlmostImmediate(
callback: (...args: any[]) => void,
...args: any[]
): AlmostImmediate {
return enableSetAlmostImmediate()
? window.setTimeout(callback, 0, ...args)
: setImmediate(callback, ...args)
}
/** Used to clear references created by setAlmostImmediate. */
export function clearAlmostImmediate(almostImmediate: AlmostImmediate) {
if (typeof almostImmediate === 'number') {
clearTimeout(almostImmediate)
} else {
clearImmediate(almostImmediate)
}
}

3
app/src/lib/stores/commit-status-store.ts
View file

@ -6,7 +6,6 @@ import { AccountsStore } from './accounts-store'
import { GitHubRepository } from '../../models/github-repository'
import { API, getAccountForEndpoint } from '../api'
import { IDisposable, Disposable } from 'event-kit'
import { setAlmostImmediate } from '../set-almost-immediate'
import {
ICombinedRefCheck,
IRefCheck,
@ -204,7 +203,7 @@ export class CommitStatusStore {
private queueRefresh() {
if (!this.refreshQueued) {
this.refreshQueued = true
setAlmostImmediate(() => {
setImmediate(() => {
this.refreshQueued = false
this.refreshEligibleSubscriptions()
})

3
app/src/lib/stores/repositories-store.ts
View file

@ -23,7 +23,6 @@ import { clearTagsToPush } from './helpers/tags-to-push-storage'
import { IMatchedGitHubRepository } from '../repository-matching'
import { shallowEquals } from '../equality'
import { enableRepositoryAliases } from '../feature-flag'
import { setAlmostImmediate } from '../set-almost-immediate'
/** The store for local repositories. */
export class RepositoriesStore extends TypedBaseStore<
@ -664,7 +663,7 @@ export class RepositoriesStore extends TypedBaseStore<
*/
private emitUpdatedRepositories() {
if (!this.emitQueued) {
setAlmostImmediate(() => {
setImmediate(() => {
this.getAll()
.then(repos => this.emitUpdate(repos))
.catch(e => log.error(`Failed emitting update`, e))

3
app/src/ui/app.tsx
View file

@ -142,7 +142,6 @@ import { AddSSHHost } from './ssh/add-ssh-host'
import { SSHKeyPassphrase } from './ssh/ssh-key-passphrase'
import { getMultiCommitOperationChooseBranchStep } from '../lib/multi-commit-operation'
import { ConfirmForcePush } from './rebase/confirm-force-push'
import { setAlmostImmediate } from '../lib/set-almost-immediate'
const MinuteInMilliseconds = 1000 * 60
const HourInMilliseconds = MinuteInMilliseconds * 60
@ -543,7 +542,7 @@ export class App extends React.Component<IAppProps, IAppState> {
}
private boomtown() {
setAlmostImmediate(() => {
setImmediate(() => {
throw new Error('Boomtown!')
})
}

11
app/src/ui/diff/image-diffs/modified-image-diff.tsx
View file

@ -8,11 +8,6 @@ import { OnionSkin } from './onion-skin'
import { Swipe } from './swipe'
import { assertNever } from '../../../lib/fatal-error'
import { ISize, getMaxFitSize } from './sizing'
import {
AlmostImmediate,
clearAlmostImmediate,
setAlmostImmediate,
} from '../../../lib/set-almost-immediate'
interface IModifiedImageDiffProps {
readonly previous: Image
@ -68,7 +63,7 @@ export class ModifiedImageDiff extends React.Component<
private container: HTMLElement | null = null
private readonly resizeObserver: ResizeObserver
private resizedTimeoutID: AlmostImmediate | null = null
private resizedTimeoutID: NodeJS.Immediate | null = null
public constructor(props: IModifiedImageDiffProps) {
super(props)
@ -80,10 +75,10 @@ export class ModifiedImageDiff extends React.Component<
// when we're reacting to a resize so we'll defer it until after
// react is done with this frame.
if (this.resizedTimeoutID !== null) {
clearAlmostImmediate(this.resizedTimeoutID)
clearImmediate(this.resizedTimeoutID)
}
this.resizedTimeoutID = setAlmostImmediate(
this.resizedTimeoutID = setImmediate(
this.onResized,
entry.target,
entry.contentRect

11
app/src/ui/history/commit-summary.tsx
View file

@ -14,11 +14,6 @@ import { wrapRichTextCommitMessage } from '../../lib/wrap-rich-text-commit-messa
import { DiffOptions } from '../diff/diff-options'
import { RepositorySectionTab } from '../../lib/app-state'
import { IChangesetData } from '../../lib/git'
import {
AlmostImmediate,
clearAlmostImmediate,
setAlmostImmediate,
} from '../../lib/set-almost-immediate'
import { TooltippedContent } from '../lib/tooltipped-content'
import { clipboard } from 'electron'
import { TooltipDirection } from '../lib/tooltip'
@ -132,7 +127,7 @@ export class CommitSummary extends React.Component<
> {
private descriptionScrollViewRef: HTMLDivElement | null = null
private readonly resizeObserver: ResizeObserver | null = null
private updateOverflowTimeoutId: AlmostImmediate | null = null
private updateOverflowTimeoutId: NodeJS.Immediate | null = null
private descriptionRef: HTMLDivElement | null = null
public constructor(props: ICommitSummaryProps) {
@ -151,10 +146,10 @@ export class CommitSummary extends React.Component<
// when we're reacting to a resize so we'll defer it until after
// react is done with this frame.
if (this.updateOverflowTimeoutId !== null) {
clearAlmostImmediate(this.updateOverflowTimeoutId)
clearImmediate(this.updateOverflowTimeoutId)
}
this.updateOverflowTimeoutId = setAlmostImmediate(this.onResized)
this.updateOverflowTimeoutId = setImmediate(this.onResized)
}
}
})

13
app/src/ui/lib/list/list.tsx
View file

@ -17,11 +17,6 @@ import { createUniqueId, releaseUniqueId } from '../../lib/id-pool'
import { range } from '../../../lib/range'
import { ListItemInsertionOverlay } from './list-item-insertion-overlay'
import { DragData, DragType } from '../../../models/drag-drop'
import {
AlmostImmediate,
clearAlmostImmediate,
setAlmostImmediate,
} from '../../../lib/set-almost-immediate'
/**
* Describe the first argument given to the cellRenderer,
@ -281,7 +276,7 @@ export class List extends React.Component<IListProps, IListState> {
private list: HTMLDivElement | null = null
private grid: Grid | null = null
private readonly resizeObserver: ResizeObserver | null = null
private updateSizeTimeoutId: AlmostImmediate | null = null
private updateSizeTimeoutId: NodeJS.Immediate | null = null
public constructor(props: IListProps) {
super(props)
@ -299,10 +294,10 @@ export class List extends React.Component<IListProps, IListState> {
// when we're reacting to a resize so we'll defer it until after
// react is done with this frame.
if (this.updateSizeTimeoutId !== null) {
clearAlmostImmediate(this.updateSizeTimeoutId)
clearImmediate(this.updateSizeTimeoutId)
}
this.updateSizeTimeoutId = setAlmostImmediate(
this.updateSizeTimeoutId = setImmediate(
this.onResized,
entry.target,
entry.contentRect
@ -779,7 +774,7 @@ export class List extends React.Component<IListProps, IListState> {
public componentWillUnmount() {
if (this.updateSizeTimeoutId !== null) {
clearAlmostImmediate(this.updateSizeTimeoutId)
clearImmediate(this.updateSizeTimeoutId)
this.updateSizeTimeoutId = null
}

106
eslint-rules/set-almost-immediate.js
View file

@ -1,106 +0,0 @@
// @ts-check
/**
* set-almost-immediate
*
* This custom eslint rule is highly specific to GitHub Desktop and attempts
* to prevent using setImmediate since, under some circumstances, could not work
* at all in Electron 11.4.0+
*
* For more info about this issue, see: https://github.com/electron/electron/issues/29261
*
* As long as this issue persists, we'll use an alternative named setAlmostImmediate,
* and this rule will ensure that's used instead of setImmediate.
*/
/**
* @typedef {import('@typescript-eslint/experimental-utils').TSESLint.RuleModule} RuleModule
* @typedef {import("@typescript-eslint/typescript-estree").TSESTree.TSTypeAnnotation} TSTypeAnnotation
* @typedef {import("@typescript-eslint/typescript-estree").TSESTree.TypeNode} TypeNode
*/
/** @type {RuleModule} */
module.exports = {
meta: {
type: 'problem',
messages: {
setImmediateForbidden: `setImmediate cannot be used, use setAlmostImmediate instead`,
clearImmediateForbidden: `clearImmediate cannot be used, use clearAlmostImmediate instead`,
nodeJSImmediateForbidden: `NodeJS.Immediate cannot be used, use AlmostImmediate instead`,
},
fixable: 'code',
schema: [],
},
create: function (context) {
const sourceCode = context.getSourceCode()
/**
* Check if a type annotation contains any references to NodeJS.Immediate
* and report them to be changed to AlmostImmediate.
*
* @param {TSTypeAnnotation} node
* @param {TypeNode} typeAnnotation
*/
function scanTypeAnnotation(node, typeAnnotation) {
if (typeAnnotation.type === 'TSTypeReference') {
const typeName = sourceCode.getText(typeAnnotation)
if (typeName === 'NodeJS.Immediate') {
context.report({
loc: typeAnnotation.loc,
messageId: 'nodeJSImmediateForbidden',
fix: fixer => {
return fixer.replaceTextRange(
typeAnnotation.range,
'AlmostImmediate'
)
},
})
}
} else if ('types' in typeAnnotation) {
for (const type of typeAnnotation.types) {
scanTypeAnnotation(node, type)
}
}
}
return {
TSTypeAnnotation(node) {
scanTypeAnnotation(node, node.typeAnnotation)
},
CallExpression(node) {
const { callee } = node
if (callee.type !== 'Identifier') {
return
}
const functionName = sourceCode.getText(callee)
const offendingFunctions = {
setImmediate: {
messageId: 'setImmediateForbidden',
replacement: 'setAlmostImmediate',
},
clearImmediate: {
messageId: 'clearImmediateForbidden',
replacement: 'clearAlmostImmediate',
},
}
const offendingCall = offendingFunctions[functionName]
if (offendingCall !== undefined) {
context.report({
node: callee,
messageId: offendingCall.messageId,
fix: fixer => {
return fixer.replaceTextRange(
node.callee.range,
offendingCall.replacement
)
},
})
}
},
}
},
}

80
eslint-rules/tests/set-almost-immediate.test.js
View file

@ -1,80 +0,0 @@
// @ts-check
const { ESLintUtils } = require('@typescript-eslint/experimental-utils')
const RuleTester = ESLintUtils.RuleTester
const rule = require('../set-almost-immediate')
// ------------------------------------------------------------------------------
// Tests
// ------------------------------------------------------------------------------
const ruleTester = new RuleTester({
parser: '@typescript-eslint/parser',
})
ruleTester.run('set-almost-immediate', rule, {
valid: [
{
filename: 'app/src/ui/diff/helper.ts',
code: `
const ref: AlmostImmediate = setAlmostImmediate(() => {})
clearAlmostImmediate(ref)
`,
},
{
filename: 'app/src/ui/some-class.ts',
code: `
class SomeClass {
private ref: AlmostImmediate
}
`,
},
],
invalid: [
{
filename: 'app/src/ui/helper.ts',
code: `
const ref: NodeJS.Immediate = setImmediate(() => {})
clearImmediate(ref)
`,
errors: [
{
messageId: 'nodeJSImmediateForbidden',
},
{
messageId: 'setImmediateForbidden',
},
{
messageId: 'clearImmediateForbidden',
},
],
output: `
const ref: AlmostImmediate = setAlmostImmediate(() => {})
clearAlmostImmediate(ref)
`,
},
{
filename: 'app/src/ui/some-class.ts',
code: `
class SomeClass {
private ref: NodeJS.Immediate
private nullableRef: NodeJS.Immediate | null
}
`,
errors: [
{
messageId: 'nodeJSImmediateForbidden',
},
{
messageId: 'nodeJSImmediateForbidden',
},
],
output: `
class SomeClass {
private ref: AlmostImmediate
private nullableRef: AlmostImmediate | null
}
`,
},
],
})