Transform arrow functions

[dfilipidisz]

Used the following command to do the transformation:

jscodeshift -t js-codemod/transforms/arrow-function.js <file> --max-width=120

I left out the following eslint rules because I don't know enough about the style direction of the project:

• arrow-body-style
• arrow-parens

[ljharb]

I think we should enable arrow-body-style as "as-needed" in this PR, and also arrow-parens as "as-needed".

The option for each rule, we can leave off for now, but those changes will be smaller, and I'd prefer to keep the changes to all the arrow fns in the same PR.

[ljharb]

Leaving a reminder to myself later to change this to .find

[ljharb]

This could really just be .map(Number)

[dfilipidisz]

I've added the 2 remaining rules without the options.

I'm sorry, I might be misunderstanding you. Should I add those options (ex.: "requireForBlockBody": true for arrow-parens) in this PR?

Also does this mean that shorthand-methods should also come in this PR?

(I'll merge from master tomorrow morning to resolve conflicts)

[ljharb]

Thanks!

Let's not add requireForBlockBody just yet.

As for shorthand-methods, that might be a good idea, but I don't think it's required. Are all these changes autofixes? It'd be ideal if all the autofixing changes went in one commit, and any manual changes in their own commits.

(Please rebase from the command line, not merge, so as to avoid merge commits)

[dfilipidisz]

Yes, all autofixes, wanted to add manual fixes in a separate commit but wasn't needed in this case AFAIK.

I'll look into how to rebase than, so far mostly used the Desktop app.

[dfilipidisz]

I failed. Here's what I did:

Updated my master (dfilipidisz/eslint-plugin-react) from origin master (yannickcr/eslint-plugin-react) based on this.

Started rebase on the PR branch:

git rebase -i master

Didn't sqash or change commit messages. (Although I think I should have changed one to be more descriptive)

There was a conflict in the void-dom-elements-no-children rule.
Somehow this part:

if (hasChildrenAttributeOrDanger) {
    // e.g. <br children="Foo" />
    context.report({
      node: node,
      message: errorMessage(elementName)
  });
}

was conflicting with this part.

if (args.length < 1) {
  // React.createElement() should not crash linter
  return;
}

And I didn't realize that those parts were in different parts of the file, and were conflicted because the line numbers changed because of my previous commits (I assume).
So I managed to keep only the first part and removed the second one to resolve the conflict, than added my change with:

git add .
git rebase --continue

And than the rebase finished.

Now the real problem was that I forgot to run npm run test before pushing my changes. Now there is an extra merge commit and an error because of the code I left out from the PR.

My question is how to proceed?

• Scrap this PR and create a new one? No manual code changes are in this PR, but than it would need to be re-reviewed I assume, which seems unnecessary.
• I think this could be fixed with another commit based on this but that would add another commit on top which was precisely the thing I was asked to avoid.

I have a backup branch from before the rebase, so I think with these steps I could fix what I've done, on the cost of another commit.

[ljharb]

You definitely don't need to ever scrap a PR!

Hm, npm test shouldn't cause or prevent a merge commit, unless you clicked the "update branch" button in github (which always creates a merge commit).

If it's too much trouble I'm happy to rebase it for you; extra commits and frequent force pushes are fine tho :-)

[dfilipidisz]

Oh okay than, if I'm not causing any major trouble I would like to try to fix this mess I made. It's definitely a learning experience for me, but it's good to know someone can help, thanks!

[ljharb]

Take your time; I'm happy to step in when needed. Always better to fix an existing PR than to create clutter by opening up a new one :-) Just make sure not to delete your remote branch, or the PR will be unrecoverable!

[dfilipidisz]

At this point I believe I'm just making matters worse. The master branch passes 2373 test and this PR passes 2363 tests when running npm run test.

I've added back the code part I know I erased while resolving badly a conflict during the first rebase.

[ljharb]

@dfilipidisz certainly this needs a rebase since it contains merge commits - if it's become untenable to do that, perhaps we reset the branch to master, rerun the codemods, and force push up the results?

[dfilipidisz]

@ljharb Okay. So before I mess up again:

1. git reset --hard master I will end up with a clean branch if I understand correctly.
2. Rerun codemods. Commit each change separately as before.
3. git push --force origin Transform-arrow-functions Will update this PR without needing to merge into master. Question here: do I want to use --force-with-lease instead to be safe?

[ljharb]

@dfilipidisz yes, all your steps look good. You can use force-with-lease but since you're the only one pushing to that branch on your fork, it doesn't really matter.

[dfilipidisz]

Re-opening. Re-run everything, test pass, nothing seems to be messed up so far.