-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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
Decaff helper jscodemods. #5732
Decaff helper jscodemods. #5732
Conversation
Thanks for the contribution! Below are some guidelines Cypress uses when doing PR reviews.
PR Review ChecklistIf any of the following requirements can't be met, leave a comment in the review selecting 'Request changes', otherwise 'Approve'. User Experience
Functionality
Maintainability
Quality
Internal
|
Failed because of a flaky test. |
Very cool. The first 2 are great, the last one scares me a little 😅 - will get @bkucera to look over since he's been heading up the decaff process. |
Added 2 more codemods. Check above. |
@sainthkh wow, this is awesome. I'll pull this down and try it out. |
It would be better to add these unit tests to CI. |
4c5ce17
to
d9b4de5
Compare
I realized that I reverted them for now. I'll revert them back when we can use them. |
958d7fd
to
d610829
Compare
still need to try this locally, sorry for the delay |
This reverts commit d610829.
662eeea
to
a3b1069
Compare
added small fix for errors I encountered during a decaf |
Test summaryRun details
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- tested locally
might push this as a separate PR so we can have parallel tests |
Going to keep a running list of minor issues I found with these codemods:
current: getBar = () ->
if foo = getFoo()
bar = 'bar'
myBar = getBar() js const getBar = () => {
let foo
foo = getBar()
if (foo) {
const bar = 'bar'
}
}
const myBar = getBar() desired: const getBar = () => {
let foo
foo = getBar()
if (foo) {
const bar = 'bar'
return bar
}
}
const myBar = getBar() |
@bkucera After a little bit of investigation, I found out that that removing It has nothing to do with this PR. Without helpers in this PR const getBar = function () {
let foo
if (foo = getFoo()) {
let bar
bar = 'bar'
}
}
const myBar = getBar() Without const getBar = function () {
let foo
if (foo = getFoo()) {
let bar
return bar = 'bar'
}
}
const myBar = getBar() Maybe we need a new codemod that does things like this. Coffee: f = () ->
foo = 'foo' Decaffeinated: f = () => {
let foo
return foo = 'foo'
} After codemod: f = () => {
let foo
foo = 'foo'
return foo
} Do you want me to make this? |
thanks @sainthkh! I think it's fine to leave as is, I'll open an issue with the decaffeinate folks. This also happened while decaffeinating, I can confirm this time it doesn't happen after commenting out the no-cond-assign codemod |
* Added a file that removes comments that start with #. * Added .eslintrc to silence ts errors in codemods. * Convert switch (false) to if-else. * Add disable-eslint no-empty by default after empty catch. * Reserve comments when converting switch-case. * Added new codemods to bulk-decaffeinate. * Preserve comments. * Ignored lint rules for test fixtures. * Move comments between arrow and block to the top. * Merged tests into one file. * Turned off no-undef for test fixtures. * No-cond-assign * Added jscodemods to decaff config. * Fixed typo. * Make CI test codemods * Added test-no-return. * Revert "Added test-no-return." This reverts commit d610829. * fix arrow-comment Co-authored-by: Ben Kucera <14625260+Bkucera@users.noreply.github.com> Co-authored-by: Zach Bloomquist <github@chary.us>
User facing changelog
N/A
Additional details
Why was this change necessary?
To decaff faster by doing repetitive and error-prone things with code.
What is affected by this change?
jest
is added to dependency asjscodeshift
only provides jest test utils.Any implementation details to explain?
N/A
How has the developer experience changed?
6 more steps are added to decaff process.
1. Remove sharp comment.
//#
->//
CoffeeScript:
## This is a comment
Decaffeinated:
//# This is a comment
After this PR:
// This is a comment
2. Add eslint-disable-line to empty catch
CoffeeScript
Decaffeinated:
After this PR:
3. switch (false) -> if-else
CoffeeScript:
Decaffeinated:
After this PR:
Note: As we all know,
switch (false)
is rarely used in human-written code. And the conversion process is really tedious and error-prone by hand because conditions are reversed.That's why I've created this mod to fix this easier and in more human-friendly way.
4. Remove comment between arrow and block
CoffeeScript:
Decaffeinated:
After this PR:
Note: Moving
{
up next to => will be done by eslint.5. no-cond-assign
CoffeeScript:
Decaffeinated:
After this PR:
6. test functions should not return.
This is reverted because mocha
it
function can return promise.PR Tasks
Has a PR for user-facing changes been opened incypress-documentation
?Have API changes been updated in thetype definitions
?Have new configuration options been added to thecypress.schema.json
?