-
Notifications
You must be signed in to change notification settings - Fork 69
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
fix(webdriverjs): prevent selnium undefined -> null transformation #402
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.
tests
packages/webdriverjs/src/browser.ts
Outdated
@@ -49,7 +49,7 @@ export function axeRunPartial( | |||
var callback = arguments[arguments.length - 1]; | |||
var context = ${JSON.stringify(context)} || document; | |||
var options = ${JSON.stringify(options)} || {}; | |||
window.axe.runPartial(context, options).then(callback); | |||
window.axe.runPartial(context, options).then(res => JSON.parse(JSON.stringify(res))).then(callback); |
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.
How does this solve the issue?
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 issue is that there are places where a key is set to undefined
rather than deleting it. Selenium finds these and converts them to null
. JSON roundtrip removed the keys
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.
Why not just return the string, so you don't have to serialise twice
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 needs a test
packages/webdriverjs/src/browser.ts
Outdated
@@ -49,7 +49,7 @@ export function axeRunPartial( | |||
var callback = arguments[arguments.length - 1]; | |||
var context = ${JSON.stringify(context)} || document; | |||
var options = ${JSON.stringify(options)} || {}; | |||
window.axe.runPartial(context, options).then(callback); | |||
window.axe.runPartial(context, options).then(res => JSON.parse(JSON.stringify(res))).then(callback); |
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.
Why not just return the string, so you don't have to serialise twice
Got rid of extra serialization. Added a test.
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.
LGTM
No description provided.