-
Notifications
You must be signed in to change notification settings - Fork 14
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: querySelector returns null
when nothing is found.
#16
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.
Other than the tests being commented out, this looks like it should be fine. Can you restore those tests?
test/web-platform-tests.js
Outdated
@@ -81,271 +81,271 @@ function read(file) { | |||
return fs.readFileSync(Path.resolve(__dirname, '..', file), 'utf8'); | |||
} | |||
|
|||
var testharness = require(__dirname + '/web-platform-tests/resources/testharness.js'); | |||
// var testharness = require(__dirname + '/web-platform-tests/resources/testharness.js'); |
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.
Please uncomment this file. While all the tests don't pass, we do have some that we need and rely on.
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'm probably missing some insights here. I was getting an error because that particular file doesn't exisit on the repo. Hense all the commented code.
Also to me that variable wasn't even used, the tests using the harness
function were comment at the end of the file !
FYI, this would be a breaking change, we should make sure we merge it once the main branch is available for v17 changes. |
@AndrewKushnir While I get that it feels like a breaking change, I thought It would be more of a fix since we have a different behavior between Browser and SSR. |
@JeanMeche agreed, this is a legit fix, but it'd still be breaking for cases where there is a strict comparison like |
Hi Angular team, do you have an update on when you think this issue will be resolved? |
Also: commenting unused tests code that prevented running the unit tests. Fixes angular/angular#51068
05028b1
to
e2a1969
Compare
Also: commenting unused test code that prevented from running the unit tests.
Fixes angular/angular#51068