-
Notifications
You must be signed in to change notification settings - Fork 783
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
Provide default arguments to axe.cleanup() #709
Conversation
lib/core/public/cleanup-plugins.js
Outdated
@@ -42,4 +42,8 @@ function cleanupPlugins(resolve, reject) { | |||
}) | |||
.catch(reject); | |||
} | |||
|
|||
function cleanupPlugins(resolve=(() => {}), reject=axe.log) { |
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.
You don't need to define a new function for this. Please just put these default params into the original function.
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.
I had to do this because of:
Warning: lib/core/public/cleanup-plugins.js: Non-simple parameter in strict mode (2:24) Use --force to continue.
In order to put default params in the original function signature, I would have to disable strict mode, which I don't think you want to do.
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 seems to be a JSHint bug. Just use fallback params instead:
resolve = resolve || () => {};
reject = reject || axe.log;
Let's be sure to squash the commits when we merge this to a) get a commit message that adheres to our commit policy, and b) get the merge commit out of there. |
@lsiden You'll have to agree to the license before we can merge in. |
@WilcoFiers I already agreed to the License Agreement as "lsiden". I think "Lawrence Siden" is from a the Github account "lsiden-deque@gmail.com" which I created when I joined Deque Labs several months ago, but just deleted minutes ago since I haven't used it. I don't know why it still show up as a "contributor" to this PR. Maybe it has been cached. |
Will do @marcysutton, once it's ready. Thank you for reminding me. |
…uments are provided Document signature of axe.cleanup()
Co-authored-by: michael-siek <michael-siek@users.noreply.github.com>
axe.cleanup() requires two functional arguments, 'resolve' and 'reject' that are not mentioned in the documentation (I will create a separate pull-request for that).
This change will define default arguments with reasonable behavior: default resolve() does nothing and default reject() logs an error message to the console.
This is related to Issue #705 .