Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[code suggestion] multiples of || into Array.includes #1610

Closed
ghost opened this issue May 18, 2017 · 4 comments
Closed

[code suggestion] multiples of || into Array.includes #1610

ghost opened this issue May 18, 2017 · 4 comments

Comments

@ghost
Copy link

ghost commented May 18, 2017

https://github.com/desktop/desktop/blob/master/app/src/models/app-menu.ts#L247-L252

export function itemMayHaveAccessKey(item: MenuItem): item is IMenuItem | ISubmenuItem | ICheckboxMenuItem | IRadioMenuItem {
  return item.type === 'menuItem' ||
    item.type === 'submenuItem' ||
    item.type === 'checkbox' ||
    item.type === 'radio'
}

maybe have it converted to:

['menuItem', 'submenuItem', 'checkbox', 'radio'].includes(item.type)

?

not sure if TS supports it though

@shiftkey
Copy link
Member

According to the Typescript sandbox:

@ghost
Copy link
Author

ghost commented May 18, 2017

yeah, i tried tsc locally -- same result, but according to microsoft/TypeScript#2340 it should have been working since TS 2.1.5

how is array of strings not a direct descendent of Array 🤔

@shiftkey
Copy link
Member

@noformnocontent Probably because we're not on ES7 yet, which seems to be a requirement.

@ghost
Copy link
Author

ghost commented May 18, 2017

yeah, switching to ES7 is probably too big of a change for this issue to be fixed

@lock lock bot locked as resolved and limited conversation to collaborators Jul 22, 2018
@desktop desktop unlocked this conversation Sep 27, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant