ronantakizawa/codereview-bench huggingface dataset:
Repository: cypress-io/cypress
File: packages/graphql/src/schemaTypes/objectTypes/gql-Mutation.ts (TypeScript)

Code before:

        return ctx.appData
      },
    })

    t.field('appCreateComponentIndexHtml', {
      type: 'App',
      args: {
        template: nonNull('String'),
      },
      description: 'Create an Index HTML file for a new component testing project',
      resolve: async (root, args, ctx) => {
        await ctx.actions.project.createComponentIndexHtml(args.template)

        return ctx.appData
      },
    })

    t.nonNull.field('generateSpecFromStory', {
      type: 'Wizard',
      description: 'Generate spec from Storybook story',
      args: {
        storyPath: nonNull('String'),
      },
      async resolve (_root, args, ctx) {
        await ctx.actions.storybook.generateSpecFromStory(args.storyPath)

        return ctx.wizardData
      },
    })

    t.field('navigationMenuSetItem', {
      type: 'NavigationMenu',
      description: 'Set the current navigation item',
      args: { type: nonNull(NavItemEnum) },
      resolve: (root, args, ctx) => {
        ctx.actions.wizard.setSelectedNavItem(args.type)

        return ctx.wizard
      },
    })

    t.field('login', {
      type: 'Query',
      description: 'Auth with Cypress Cloud',
      async resolve (_root, args, ctx) {
        await ctx.actions.auth.login()

        return {}
      },
    })

Review comment (suggestion):
One comment - I think in general we probably want to return `null` instead of erroring in Mutations (I'm still learning the best practices around GraphQL too, my understanding is returning null here is more common).

Probably okay for now, good to keep in mind.

Code after:

        return ctx.appData
      },
    })

    t.field('appCreateComponentIndexHtml', {
      type: 'App',
      args: {
        template: nonNull('String'),
      },
      description: 'Create an Index HTML file for a new component testing project',
      resolve: async (root, args, ctx) => {
        await ctx.actions.project.createComponentIndexHtml(args.template)

        return ctx.appData
      },
    })

    t.nonNull.field('generateSpecFromStory', {
      type: 'Project',
      description: 'Generate spec from Storybook story',
      args: {
        storyPath: nonNull('String'),
      },
      async resolve (_root, args, ctx) {
        if (!ctx.activeProject) {
          throw Error(`Cannot set spec without active project!`)
        }

        await ctx.actions.storybook.generateSpecFromStory(args.storyPath)

        return ctx.activeProject
      },
    })

    t.field('navigationMenuSetItem', {
      type: 'NavigationMenu',
      description: 'Set the current navigation item',
      args: { type: nonNull(NavItemEnum) },
      resolve: (root, args, ctx) => {
        ctx.actions.wizard.setSelectedNavItem(args.type)

        return ctx.wizard
      },
    })

    t.field('login', {
      type: 'Query',
      description: 'Auth with Cypress Cloud',
      async resolve (_root, args, ctx) {
        await ctx.actions.auth.login()
Repository: cypress-io/cypress
File: packages/data-context/src/actions/WizardActions.ts (TypeScript)

Code before:
          this.navigateToStep('initializePlugins')
        }
      }

      if (this.data.chosenTestingType === 'e2e') {
        if (this.ctx.activeProject?.e2ePluginsInitialized) {
          this.navigateToStep('setupComplete')
        } else {
          this.navigateToStep('initializePlugins')
        }
      }
    }

    return this.data
  }

  /// reset wizard history, useful for when changing to a new project
  resetWizard () {
    this.data.currentStep = 'welcome'
    this.data.history = []
    this.data.chosenBundler = null
    this.data.chosenTestingType = null
    this.data.chosenFramework = null
    this.data.chosenLanguage = 'js'
    this.data.chosenManualInstall = false
    this.data.chosenBrowser = null

    return this.data
  }
}


Review comment (suggestion):
Oh this is where we do the browser reset to `null`, I think it would be OK to not reset this at all between projects. The most recent choice seems a valid starting point for that option. I think the goal in the UI is that if the preselected browser is fine, you just smoothly click on Launch without having to do anything.

Code after:
          this.navigateToStep('initializePlugins')
        }
      }

      if (this.data.chosenTestingType === 'e2e') {
        if (this.ctx.activeProject?.e2ePluginsInitialized) {
          this.navigateToStep('setupComplete')
        } else {
          this.navigateToStep('initializePlugins')
        }
      }
    }

    return this.data
  }

  /// reset wizard history, useful for when changing to a new project
  resetWizard () {
    this.data.currentStep = 'welcome'
    this.data.history = []
    this.data.chosenBundler = null
    this.data.chosenTestingType = null
    this.data.chosenFramework = null
    this.data.chosenLanguage = 'js'
    this.data.chosenManualInstall = false

    return this.data
  }
}
Repository: cypress-io/cypress
File: packages/app/src/pages/NewSpec.vue (Vue)

Code before:
}
`

gql`
mutation NewSpec_GenerateSpecFromStory($storyPath: String!) {
  generateSpecFromStory (storyPath: $storyPath) {
    storybook {
      generatedSpec {
        content
        spec {
          id
          name
          relative
        }
      }
    }
  }
} 
`

const query = useQuery({ query: NewSpecQueryDocument })
const mutation = useMutation(NewSpec_GenerateSpecFromStoryDocument)

const generatedSpec = computed(() => mutation.data.value?.generateSpecFromStory.storybook?.generatedSpec)

async function storyClick (story: string) {
  await mutation.executeMutation({ storyPath: story })
}

function specClick () {
  window.location.href = `${window.location.origin}/__/#/tests/component/${generatedSpec.value?.spec.name}`
}

const stories = computed(() => {
  return query.data.value?.app.activeProject?.storybook?.stories?.edges.map(({ node: story }) => {
    return {
      ...story,
      fileExtension: story.baseName.replace(story.fileName, ''),
      relativeFromProjectRoot: story.relative.replace(story.baseName, ''),
    }
  }) || []
})

</script>


Review comment (suggestion):
Any reason for this to be `async` and to `await` the mutation? Seems like this would be the same:


```ts
function storyClick (story: string) {
  mutation.executeMutation({ storyPath: story })
}

Code after:
}
`

gql`
mutation NewSpec_GenerateSpecFromStory($storyPath: String!) {
  generateSpecFromStory (storyPath: $storyPath) {
    storybook {
      generatedSpec {
        content
        spec {
          id
          name
          relative
        }
      }
    }
  }
} 
`

const query = useQuery({ query: NewSpecQueryDocument })
const mutation = useMutation(NewSpec_GenerateSpecFromStoryDocument)

const generatedSpec = computed(() => mutation.data.value?.generateSpecFromStory.storybook?.generatedSpec)

function storyClick (story: string) {
  mutation.executeMutation({ storyPath: story })
}

function specClick () {
  window.location.href = `${window.location.origin}/__/#/tests/component/${generatedSpec.value?.spec.name}`
}

const stories = computed(() => {
  return query.data.value?.app.activeProject?.storybook?.stories?.edges.map(({ node: story }) => {
    return {
      ...story,
      fileExtension: story.baseName.replace(story.fileName, ''),
      relativeFromProjectRoot: story.relative.replace(story.baseName, ''),
    }
  }) || []
})

</script>
Repository: cypress-io/cypress
File: packages/graphql/src/schemaTypes/objectTypes/gql-Mutation.ts (TypeScript)

Code before:
        testingType: nonNull(TestingTypeEnum),
        browserId: nonNull(idArg({
          description: 'ID of the browser that we want to set',
        })),
      },
      async resolve (_, args, ctx) {
        await ctx.actions.project.setProjectPreferences(args)

        return ctx.appData
      },
    })

    t.nonNull.field('hideBrowserWindow', {
      type: 'App',
      description: 'Hides the launchpad windows',
      resolve: (_, args, ctx) => {
        ctx.actions.electron.hideBrowserWindow()

        return ctx.appData
      },
    })

    t.nonNull.field('showBrowserWindow', {
      type: 'App',
      description: 'show the launchpad windows',
      resolve: (_, args, ctx) => {
        ctx.actions.electron.showBrowserWindow()

        return ctx.appData
      },
    })
  },
})


Review comment (suggestion):
I wonder if these two should be booleans like the internal one since we don't really care what they return

Code after:
        if (!ctx.activeProject) {
          throw Error(`Cannot set spec without active project!`)
        }

        await ctx.actions.project.setCurrentSpec(args.id)
      },
    })

    t.nonNull.field('setProjectPreferences', {
      type: 'App',
      description: 'Save the projects preferences to cache',
      args: {
        testingType: nonNull(TestingTypeEnum),
        browserId: nonNull(idArg({
          description: 'ID of the browser that we want to set',
        })),
      },
      async resolve (_, args, ctx) {
        await ctx.actions.project.setProjectPreferences(args)

        return ctx.appData
      },
    })

    t.nonNull.field('hideBrowserWindow', {
      type: 'App',
      description: 'Hides the launchpad windows',
      resolve: (_, args, ctx) => {
        ctx.actions.electron.hideBrowserWindow()

        return ctx.appData
      },
    })

    t.nonNull.field('showBrowserWindow', {
      type: 'App',
      description: 'show the launchpad windows',
      resolve: (_, args, ctx) => {
        ctx.actions.electron.showBrowserWindow()

        return ctx.appData
      },
    })
  },
})
Repository: cypress-io/cypress
File: packages/types/src/cache.ts (TypeScript)

Code before:
export type Cache = {
  PROJECTS: string[]
  PROJECT_PREFERENCES: ProjectPreferences
  USER: CachedUser
}

export type ProjectPreferences = { [project: string]: Preferences}

export type Preferences = {
  browserId: string | null
  testingType: 'e2e' | 'component' | null
}

export type CachedUser = {
  authToken: any
  name: any
  email: any
}


Review comment (suggestion):
I think the general pattern for object definitions is `interface` looking at the rest of `@packages/types`. `type` seems more commonly used for unions and intersections of multiple types/interfaces.

Code after:
export interface Cache {
  PROJECTS: string[]
  PROJECT_PREFERENCES: Record<string, Preferences>
  USER: CachedUser
}

export interface Preferences {
  browserId: string | null
  testingType: 'e2e' | 'component' | null
}

export interface CachedUser {
  authToken: string
  name: string
  email: string
}
Repository: cypress-io/cypress
File: packages/frontend-shared/src/components/CopyButton.vue (Vue)

Code before:
      tabindex="-1"
      :value="text"
      class="absolute -top-96"
    />
  </div>
</template>

<script setup lang="ts">
import { ref } from 'vue'
import { useI18n } from '@cy/i18n'
import Button from '../components/Button.vue'
import TransitionQuickFade from '../components/transitions/TransitionQuickFade.vue'

defineProps<{
  text: string
}>()

const showCopied = ref(false)
const textElement = ref<HTMLTextAreaElement | null>(null)
const copyToClipboard = async () => {
  textElement.value?.select()
  document.execCommand('copy')
  showCopied.value = true
  setTimeout(() => {
    showCopied.value = false
  }, 2000)
}
const { t } = useI18n()
</script>


Review comment (refactor):
[Replace with useClipboard from @vueuse/core](https://vueuse.org/core/useclipboard/#useclipboard). See ShikiHighlight.vue.

```vue
<template>
  <div>{{ copied ? 'Copied!' : 'Copy' }}</div>
  <div ref="codeEl" @click="copyCode">console.log()</div>
</template>
```
```tsx
import { ref } from 'vue'

// eslint-disable-next-line duplicate-imports
import type { Ref } from 'vue'
import { useClipboard } from '@vueuse/core'
const { copy, copied } = useClipboard()
const codeEl:  Ref<HTMLTextAreaElement | null> = ref(null)

const copyCode = () => {
  if (codeEl.value) {
    const text = codeEl.value.innerText

    copy(text)
  }
}
```

Code after:
<template>
  <div>
    <Button
      size="md"
      variant="tertiary"
      @click="copyToClipboard"
    >
      <template #prefix>
        <i-cy-copy-clipboard_x16 class="icon-dark-indigo-500 w-16px h-16px" />
      </template>
      <TransitionQuickFade mode="out-in">
        <span v-if="!copied">{{ t('clipboard.copy') }}</span>
        <span v-else>{{ t('clipboard.copied') }}!</span>
      </TransitionQuickFade>
    </Button>
  </div>
</template>

<script setup lang="ts">
import { useClipboard } from '@vueuse/core'
import { useI18n } from '@cy/i18n'
import Button from '../components/Button.vue'
import TransitionQuickFade from '../components/transitions/TransitionQuickFade.vue'

const props = defineProps<{
  text: string
}>()

const { copy, copied } = useClipboard({ copiedDuring: 2000 })
const copyToClipboard = () => {
  if (props.text) {
    copy(props.text)
  }
}
const { t } = useI18n()
</script>
Repository: cypress-io/cypress
File: packages/app/src/layouts/default.vue (Vue)

Code before:
            {{ route.name }}
          </h1>
          <transition
            name="fade"
            mode="out-in"
          >
            <!-- <keep-alive> -->
            <component :is="Component" />
            <!-- </keep-alive> -->
          </transition>
        </router-view>
        <ModalManager v-if="modalStore.activeModalId" />
      </section>
    </main>
    <nav
      class="h-screen order-first"
      :class="mainStore.navBarExpanded ? 'w-240px' : 'w-80px'"
    >
      <SidebarNavigation class="h-full" />
    </nav>
  </div>
</template>

<script lang="ts" setup>
import SidebarNavigation from '../navigation/SidebarNavigation.vue'
import HeaderBar from '../../../frontend-shared/src/gql-components/HeaderBar.vue'
import { useRoute } from 'vue-router'
import { computed } from 'vue'

import ModalManager from '../modals/ModalManager.vue'
import { useModalStore, useMainStore } from '../store'

const modalStore = useModalStore()
const mainStore = useMainStore()
const currentRoute = useRoute()

const showHeader = computed(() => {
  // if there's more than one place in the app that will not have a header,
  // this can get more formal
  return currentRoute.name !== 'Spec'
})
</script>


Review comment (suggestion):
```suggestion
import HeaderBar from '@cy/gql-components/HeaderBar.vue'
```

with the proper alias in the tsconfig

Code after:
            {{ route.name }}
          </h1>
          <transition
            name="fade"
            mode="out-in"
          >
            <!-- <keep-alive> -->
            <component :is="Component" />
            <!-- </keep-alive> -->
          </transition>
        </router-view>
        <ModalManager v-if="modalStore.activeModalId" />
      </section>
    </main>
    <nav
      class="h-screen order-first"
      :class="mainStore.navBarExpanded ? 'w-240px' : 'w-80px'"
    >
      <SidebarNavigation class="h-full" />
    </nav>
  </div>
</template>

<script lang="ts" setup>
import SidebarNavigation from '../navigation/SidebarNavigation.vue'
import HeaderBar from '@cy/gql-components//HeaderBar.vue'
import { useRoute } from 'vue-router'
import { computed } from 'vue'

import ModalManager from '../modals/ModalManager.vue'
import { useModalStore, useMainStore } from '../store'

const modalStore = useModalStore()
const mainStore = useMainStore()
const currentRoute = useRoute()

const showHeader = computed(() => {
  return currentRoute.meta.header !== false
})

const pageName = computed((): string | undefined => {
  return currentRoute.meta?.title as string
})
</script>
Repository: cypress-io/cypress
File: packages/app/src/pages/Index.vue (Vue)

Code before:
<template>
  <div>Home page</div>
</template>

<route>
{
  name: "Home"
}
</route>


Review comment (suggestion):
Can you remove those changes from the PR? 
They will probably conflict with other changes in different Tickets

Code after:
<template>
  <div>Home page</div>
</template>

<route>
{
  name: "Home Page",
  meta: {
    title: "Home"
  }
}
</route>
Repository: cypress-io/cypress
File: packages/frontend-shared/src/gql-components/HeaderBar.vue (Vue)

Code before:
<template>
  <HeaderBarContent
    v-if="query.data.value"
    :gql="query.data.value"
    :show-browsers="props.showBrowsers"
    :page-name="pageName"
  />
</template>

<script setup lang="ts">
import { gql, useQuery } from '@urql/vue'
import HeaderBarContent from './HeaderBarContent.vue'
import { HeaderBar_HeaderBarQueryDocument } from '../generated/graphql'
import { useI18n } from '@cy/i18n'
import type { RouteRecordName } from 'vue-router'

gql`
query HeaderBar_HeaderBarQuery {
  ...HeaderBar_HeaderBarContent
}
`

const props = defineProps<{
    showBrowsers?: Boolean,
    pageName?: string
}>()

const { t } = useI18n()

const query = useQuery({ query: HeaderBar_HeaderBarQueryDocument })

</script>


Review comment (suggestion):
I think you want `boolean` (typescript type) as opposed to `Boolean` (javascript constructor)

Code after:
<template>
  <HeaderBarContent
    v-if="query.data.value"
    :gql="query.data.value"
    :show-browsers="props.showBrowsers"
    :page-name="pageName"
  />
</template>

<script setup lang="ts">
import { gql, useQuery } from '@urql/vue'
import HeaderBarContent from './HeaderBarContent.vue'
import { HeaderBar_HeaderBarQueryDocument } from '../generated/graphql'
import { useI18n } from '@cy/i18n'

gql`
query HeaderBar_HeaderBarQuery {
  ...HeaderBar_HeaderBarContent
}
`

const props = defineProps<{
    showBrowsers?: boolean,
    pageName?: string
}>()

const { t } = useI18n()

const query = useQuery({ query: HeaderBar_HeaderBarQueryDocument })

</script>
Repository: cypress-io/cypress
File: packages/app/src/store/aut-store.ts (TypeScript)

Code before:
import { defineStore } from 'pinia'

interface AutStoreState {
  highlightUrl: boolean
  url?: string
}

export const useAutStore = defineStore({
  id: 'aut-store',
  state: (): AutStoreState => {
    return {
      highlightUrl: false,
    }
  },
  actions: {
    setHighlightUrl (highlightUrl: boolean) {
      this.highlightUrl = highlightUrl
    },
    updateUrl (url?: string) {
      this.url = url
    },
  },
})


Review comment (suggestion):
I don't think we have a great way to serve the config from GraphQL yet, but once we do, we should be able to query for the default viewport dimensions and put them here.

Code after:
import { defineStore } from 'pinia'

interface AutStoreState {
  url?: string
  highlightUrl: boolean
  viewportWidth: number
  viewportHeight: number
  isLoadingUrl: boolean
  isLoading: boolean
  isRunning: boolean
  viewportUpdateCallback: (() => void) | null
}

export const useAutStore = defineStore({
  id: 'aut-store',
  state: (): AutStoreState => {
    // TODO(lachlan): depends on CT or E2E, should seed accordingly using `config`
    return {
      isLoadingUrl: false,
      highlightUrl: false,
      url: undefined,
      viewportHeight: 660,
      viewportWidth: 1000,
      isLoading: false,
      isRunning: false,
      viewportUpdateCallback: null,
    }
  },

  actions: {
    setHighlightUrl (highlightUrl: boolean) {
      this.highlightUrl = highlightUrl
    },

    updateUrl (url?: string) {
      this.url = url
    },

    updateDimensions (viewportWidth: number, viewportHeight: number) {
      this.viewportHeight = viewportHeight
      this.viewportWidth = viewportWidth
    },
Repository: cypress-io/cypress
File: packages/launchpad/cypress/e2e/integration/choose-a-browser.spec.ts (TypeScript)

Code before:
import type { FoundBrowser } from '@packages/types/src'

describe('Choose a Browser Page', () => {
  // Walks through setup pages to get to browser selection
  const stepThroughConfigPages = () => {
    cy.get('h1').should('contain', 'Configuration Files')
    cy.contains('Continue').click()
    cy.get('h1').should('contain', 'Initializing Config...')
    cy.contains('Next Step').click()
  }

  // Mocks server's browser detection with known list
  const setupMockBrowsers = function () {
    cy.withCtx(async (ctx, o) => {
      if (ctx._apis.appApi.getBrowsers.restore) {
        ctx._apis.appApi.getBrowsers.restore()
      }

      const mockBrowsers = [{
        'id': '1',
        'channel': 'stable',
        'disabled': false,
        'isSelected': true,
        'displayName': 'Chrome',
        'family': 'chromium',
        'majorVersion': '1',
        'name': 'chrome',
        'path': '/test/chrome/path',
        'version': '1.2.333.445',
      } as FoundBrowser, {
        'id': '2',
        'channel': 'stable',
        'disabled': false,
        'isSelected': false,
        'displayName': 'Firefox',
        'family': 'firefox',
        'majorVersion': '2',
        'name': 'firefox',

Review comment (suggestion):
Could accept a parameter which defaults to an empty list of browsers for the zero mock browser case and/or defaults to mockBrowsers list for the non-zero case

Code after:
describe('Choose a Browser Page', () => {
  // Walks through setup pages to get to browser selection
  const stepThroughConfigPages = () => {
    cy.get('h1').should('contain', 'Configuration Files')
    cy.contains('button', 'Continue').click()
    cy.get('h1').should('contain', 'Initializing Config...')
    cy.contains('button', 'Next Step').click()
  }

  beforeEach(() => {
    cy.scaffoldProject('launchpad')
  })

  describe('System Browsers Detected', () => {
    beforeEach(() => {
      cy.findBrowsers({
        filter: (browser) => {
          return Cypress._.includes(['chrome', 'firefox', 'electron', 'edge'], browser.name) && browser.channel === 'stable'
        },
      })
    })

    it('preselects browser that is provided through the command line', () => {
      cy.openProject('launchpad', ['--e2e', '--browser', 'edge'])

      cy.visitLaunchpad()
      stepThroughConfigPages()

      cy.get('h1').should('contain', 'Choose a Browser')

      cy.findByRole('radio', { name: 'Edge v8', checked: true })
    })

    it('shows warning when launched with --browser name that cannot be matched to found browsers', () => {
      cy.openProject('launchpad', ['--e2e', '--browser', 'doesNotExist'])
      cy.visitLaunchpad()

      stepThroughConfigPages()