Proposal: Convert codebase CoffeeScript => JS => TypeScript

[jennifer-shehane]

Goal

Make Cypress' open source repo more accessible to contributors. Improve code quality by implementing types.

Implementation

• Add decaffeinate and bulk-decaffeinate to start conversion to JS (Add decaffeinate and bulk-decaffeinate to start conversion to JS #2686)
• TBD

[sainthkh]

Decaffeination Guide

Use this checklist to make converted JS code more readable.

Summary:

- [ ] Remove unused variables, arguments, functions
- [ ] Convert to arrow functions
- [ ] Fix weird conditional expressions
- [ ] Move variables close to where they're used
- [ ] Change `switch` to `if` if it has only one case (with `default`)
- [ ] Drag down comments
- [ ] Remove unnecessary `return`s.
- [ ] Remove functions when they're used to assign a value

Copy above to check your code.

Remove unused variables, arguments, functions

As CoffeeScript doesn't show which names are unused, we can find a lot of unused variables, arguments, or functions.

👎 Bad:

const func = (a, b) => {
    const c = 'hi?'

    return a + 3
}

const unusedFunc = (a, b, c) => {
    // something complicated going on.
}

const d = func(3)

👍 Good:

const func = (a) => {
    return a + 3
}

const d = func(3)

Covert to arrow functions

jscodemod converts some of them, but it's not perfect. So, read the code and a function doesn't have a this inside, convert it to an arrow function.

👎 Bad:

const f = function (a, b) {
    // do something
}

👍 Good:

const f = (a, b) => {
    // do something
}

Note: If you're a VS Code user, you can do this faster with JS Refactor plugin. Press Ctrl + Shift + J A. When pressing A, you should not press ctrl and shift.

Fix weird conditional expressions

Converted JS code of CoffeeScript ?. syntax is really weird. So, we need to read JS code and original CoffeeScript code carefully and convert them manually.

👎 Bad:

x = $el?.length ? 0

const x = ($el != null ? $el.length : undefined) != null ? ($el != null ? $el.length : undefined) : 0

👍 Good:

const x = $el != null ? $el.length
// or
const x = $el ? $el.length

Note: There are 2 answers here because ?. ignores falsey values. So, check the type of variable carefully and how it is used.

Move variables close to where it is used.

Sometimes, converted JavaScript variables are at the top of the function when those are used at the bottom of the function. Move them near to the line it is used.

👎 Bad:

const f = () => {
    let x;

    // Do something long here.

    if (x > 10) {
        x = 30;
    } else {
        x = 3;
    }
}

👍 Good:

const f = () => {
    // Do something long here.

    let x;

    if (x > 10) {
        x = 30;
    } else {
        x = 3;
    }
}

Change switch to if if it has only one case (with default)

It's really weird to read a switch statement with a single case (+ default).

👎 Bad:

switch(type) {
    case 'dom':
        // do something
    default:
        // do else
}

👍 Good:

if (type === 'dom') {
    // do something
} else {
    // do else
}

Drag down comments

Sometimes, comments are next to if conditions. Bring them down.

👎 Bad:

if (cond) { // Comment 1 and the tester's stone
    doSomething()
} else { // Comment 2 and the chamber of tests
    doElse()
}

👍 Good:

if (cond) { 
    // Comment 1 and the tester's stone
    doSomething()
} else { 
    // Comment 2 and the chamber of tests
    doElse()
}

Remove unnecessary returns.

Especially, in the converted test codes, there are unnecessary returns. Let's remove them.

👎 Bad:

beforeEach(() => {
    return cy.visit('/some-web-page')
})

👍 Good:

beforeEach(() => {
    cy.visit('/some-web-page')
})

EDIT on 4/14/2020

When the return is used in test cases, you should be careful. Because mocha uses return to check when async tests end.

To remove it, you need to check the type of the returned value. If it's a promise, don't remove return.

Remove functions when they're used to assign a value

👎 Bad:

const type = (() => {
  if (env.get('CYPRESS_CI_KEY')) {
    return 'CYPRESS_CI_DEPRECATED_ENV_VAR'
  }

  return 'CYPRESS_CI_DEPRECATED'
})()

This is the translation of CoffeeScript when you assign a value directly from switch statement like below:

type = switch
  when env.get("CYPRESS_CI_KEY")
    "CYPRESS_CI_DEPRECATED_ENV_VAR"
  else
    "CYPRESS_CI_DEPRECATED"

👍 Good:

let type = 'CYPRESS_CI_DEPRECATED'

if (env.get('CYPRESS_CI_KEY')) {
  type = 'CYPRESS_CI_DEPRECATED_ENV_VAR'
}

If you have any idea to improve this list, please leave me a comment or directly fix this.

[jennifer-shehane]

Instructions for decaffeinating files

1. The easiest way to convert is to run our VSCode tasks defined here: https://github.com/cypress-io/cypress/blob/issue-6825/.vscode/tasks.json#L11:L11 You just to CTRL+SHIFT+P- type Task then type the one you want to run, whether you want to convert a single file, multiple files, or a whole directory.
2. This task creates 3 commits prepended with decaffeinate: (as shown in this PR: [WIP] Decaffeinate updater.js #6784) the main point of these is to maintain the git history of the file for future reference - it will also leave the older CoffeeScript file for reference so you can compare during cleanup but this needs to be deleted before committing again.
3. The file will inevitably need ‘cleanup’. @sainthkh wrote up a general decaffeination cleanup guide to follow here: Proposal: Convert codebase CoffeeScript => JS => TypeScript #2690 (comment) Your cleanup should be one commit only, so if you need to make changes after committing, make sure it ends up being one commit. Do not merge develop in after this as it will bork the commit history also.
4. Merge the PR in - NEVER SQUASH - this should commit at the most 4 commits directly to develop. (We squash every other PR which is why this is sometimes good to explicitly state in the main PR comment itself).

[jennifer-shehane]

I'm going to close this as resolved. The conversion to TypeScript is not complete, but I think this is less pressing and will come over time.

I don't think there's any CoffeeScript left (aside from testing that coffeescript works in Cypress). It's certainly less than 1% on GitHub's chart.