cypress-example-kitchensink icon indicating copy to clipboard operation
cypress-example-kitchensink copied to clipboard

Support @typescript-eslint/no-unused-vars rule

Open karlhorky opened this issue 4 years ago • 3 comments

User facing changelog

This disables the ESLint @typescript-eslint/no-unused-vars rule (the typed counterpart to ESLint's built-in no-unused-vars)

Additional details

The files scaffolded by Cypress create an error out of the box for those users who are using @typescript-eslint/no-unused-vars instead of no-unused-vars

This rule is also possible (and desirable) to use with JS files as well - it offers additional type information even in JS files so that the rule catches more problems.

How has the user experience changed?

Before: Error out of the box

Screen Shot 2021-07-21 at 22 02 48

After: No error

Screen Shot 2021-07-21 at 22 03 34

Original PR: https://github.com/cypress-io/cypress/pull/17432

cc @flotwig

karlhorky avatar Jul 22 '21 21:07 karlhorky

CLA assistant check
All committers have signed the CLA.

CLAassistant avatar Jul 22 '21 21:07 CLAassistant

✔️ Deploy Preview for cypress-example-kitchensink ready!

🔨 Explore the source changes: 7913c0ac2136f87c9eaf9805d753f0963111faf9

🔍 Inspect the deploy log: https://app.netlify.com/sites/cypress-example-kitchensink/deploys/60f9e9d51497f80008c203e5

😎 Browse the preview: https://deploy-preview-502--cypress-example-kitchensink.netlify.app

netlify[bot] avatar Jul 22 '21 21:07 netlify[bot]



Test summary

5 0 0 0


Run details

Project cypress-example-kitchensink
Status Passed
Commit 7913c0ac21
Started Jul 22, 2021 9:59 PM
Ended Jul 22, 2021 10:02 PM
Duration 03:37 💡
OS Linux Ubuntu - 16.04
Browser Custom chromium 90

View run in Cypress Dashboard ➡️


This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard

cypress[bot] avatar Jul 22 '21 22:07 cypress[bot]

@emilyrohrbough

"This branch has conflicts that must be resolved".

This PR changes a file which no longer exists under Cypress 10+

Did you approve it by accident? Can we check that the issue it was addressing still exists?

MikeMcC399 avatar Apr 07 '23 14:04 MikeMcC399

cypress/plugins/index.js is created by default after starting Cypress 9.7.0 with

// eslint-disable-next-line no-unused-vars
module.exports = (on, config) => {
  // `on` is used to hook into various events Cypress emits
  // `config` is the resolved Cypress config
}

The file remains after updating to Cypress 12.9.0 and then running the migration wizard.

After a clean installation of Cypress 12.9.0, opening this version and running E2E scaffolding, no file cypress/plugins/index.js is created. There is no other file with the above problematic code snippet.

MikeMcC399 avatar Apr 10 '23 07:04 MikeMcC399

The issue targeted by this PR is obsolete and the PR can be closed. There is nothing to fix anymore.

Modifications to examples in this repo (cypress-io/cypress-example-kitchensink) only flow into future versions of Cypress. They do not affect released versions or existing installations.

The issue only affects installations of Cypress Legacy configuration: Folders/files which were originally installed using Cypress 9 and lower.

Installations using Cypress current Configuration: Folders/files which were originally installed using Cypress 10+ are not affected. There is no file cypress/plugins/index.js created.

MikeMcC399 avatar Apr 10 '23 07:04 MikeMcC399

This PR should be closed because it is obsolete, but also because it would not have worked even if it had been dealt with in a timely manner.

Perhaps @karlhorky or one of the Cypress.io team could close it?

Rebasing

Attempting to rebase the branch https://github.com/karlhorky/cypress-example-kitchensink/tree/patch-2 on the master branch results in a conflict:

$ git rebase master pr/502
CONFLICT (modify/delete): cypress/plugins/index.js deleted in HEAD and modified in 7913c0a (Support @typescript-eslint/no-unused-vars rule).  Version 7913c0a (Support @typescript-eslint/no-unused-vars rule) of cypress/plugins/index.js left in tree.
error: could not apply 7913c0a... Support @typescript-eslint/no-unused-vars rule
hint: Resolve all conflicts manually, mark them as resolved with
hint: "git add/rm <conflicted_files>", then run "git rebase --continue".
hint: You can instead skip this commit: run "git rebase --skip".
hint: To abort and get back to the state before "git rebase", run "git rebase --abort".
Could not apply 7913c0a... Support @typescript-eslint/no-unused-vars rule

Since there is no other change in the PR apart from a modification to the deleted file cypress/plugins/index.js, after resolving the conflict, there is nothing left in the PR.

Effectiveness

It seems also that there was a misconception that changing cypress/plugins/index.js in this repository would affect the file delivered by installing and running a legacy Cypress version (9 or earlier). In fact although this file was packaged up and included in the npm module cypress-example-kitchensink, the Cypress build process described in Updating the example app using yarn workspace @packages/example build did not copy this file. See

https://github.com/cypress-io/cypress/blob/d681fd929bfeb4ca297d42ad77fc3d0e33dae793/packages/example/bin/convert.js#L44-L50

   glob('./cypress/integration/**/*.js', { realpath: true }, (err, specFiles) => {
    if (err) throw err

    htmlFiles.concat(specFiles).forEach(function (file) {
      return replaceStringsIn(file)
    })
  })

So even if Cypress were still using the layout which is now legacy, the PR would not have had the desired effect.

MikeMcC399 avatar Apr 30 '23 10:04 MikeMcC399

Yeah I'm thinking also that this PR is no longer needed.

If I see this happening in the latest versions, I'll open a new PR.

karlhorky avatar May 03 '23 07:05 karlhorky

@karlhorky

Thanks for closing! I'm going through some of the eslint aspects at the moment, so please flag any issues if you find any new ones.

MikeMcC399 avatar May 03 '23 07:05 MikeMcC399