-
-
Notifications
You must be signed in to change notification settings - Fork 13
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
feat: create a JS helpers class #190
Conversation
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.
The first thing that stands out is that this would break for valid single-parameter syntax:
const myFunc = name => console.log("Name");
Did I get it, as all tests seem to pass? |
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.
Hi @a2937, thanks for putting this together. The code looks like it does the job well, thanks. The only issue is that it's a little hard to read, so I made some suggestions for how that could be improved:
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.
This looks great. Thanks @a2937!
@naomi-lgbt If you're happy, merge away.
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.
Ship it
Checklist:
Update index.md
)Closes #XXXXX
In my opinion, it does come up fairly often that we use regex to ensure that variables are actually present in functions with default values; and I think the potential spacing issues aren't generally worth with it when we can just manually extract them.
I also tried to do some house-keeping and hopefully keep eslint from reformatting the fixtures as it is very space sensitive. As I use Visual Studio to edit Typescript, I also added some of the contents of VisualStudio.gitignore to the local gitignore. I will be be happy to remove the unrelated content and discuss them at a later date.