search

mainmatter / qunit-dom

High Level DOM Assertions for QUnit
MIT License
179 stars 123 forks source link

assert.dom(Element).exists() throws even if it exists #245

Closed dwilhelmi closed 4 years ago

dwilhelmi commented 5 years ago

Even though officially the dom method accepts the Element type, when using it that way the exists method ends up throwing an error, even if the element exists. At root, this is because exists calls findElements, which is not currently set up to check for this.target being anything other than a string.

I have a PR inbound shortly to add support to findElements for having Element targets, but wanted to track it here also.

Turbo87 commented 5 years ago

hmm, interesting, I was pretty sure I implemented it that way at first. Has this possibly changed lately?

dwilhelmi commented 5 years ago

Not sure - findElement currently supports Element targets, but findElements does not. As promised, I have pushed up a PR with a possible fix - https://github.com/simplabs/qunit-dom/pull/246

Turbo87 commented 5 years ago

thinking some more about it, why do you use assert.dom(element).exists() instead of just assert.ok(element)? wouldn't the latter be easier?

Even though officially the dom method accepts the Element type

it does, but not all assertions support it. just like with e.g. chai when you expect(5) you can't use something like .to.contain(4).

patocallaghan commented 5 years ago

I just came up against this issue using assert.dom(element).isNotVisible(), findElements doesn't allow using an HTMLElement

step2yeung commented 5 years ago

Bumping this... We see the error with assert.dom(element).isVisible() @dwilhelmi will you have time to wrap up the PR? If not, would you care if I take it over?

monovertex commented 5 years ago

I also encountered this with isNotVisible(). Could the proposed PR be merged or is there any other work left to be done on it?

scalvert commented 5 years ago

Just to clarify, are folks encountering this issue using Typescript in their projects, and they're getting a type mismatch?

monovertex commented 5 years ago

I’m getting this in an Ember project, not using Typescript.

patocallaghan commented 5 years ago

@scalvert it's been a while since I've commented but we don't use TypeScript

scalvert commented 5 years ago

Let me take a peek again and see if there's anything outstanding we need on that PR.

scalvert commented 5 years ago

Sorry that everyone's been encountering this issue. I'm working on it now, and just adding a number of test cases to ensure that our fix is working.

scalvert commented 5 years ago

Ready to go! Just waiting for a merge/release. Thanks for your patience, everyone!