Fix typing for hasProperty assertion
#782
Conversation
wuron
force-pushed
the
fix-has-property-typing
branch
from
6fc2987 to
df9d327
Compare
wuron
force-pushed
the
fix-has-property-typing
branch
from
df9d327 to
fdb293e
Compare
|
So what do we need to merge this? |
wuron
force-pushed
the
fix-has-property-typing
branch
from
fdb293e to
9352b8d
Compare
| * @param {string?} message | ||
| * | ||
| * @example | ||
| * assert.dom('input.password-input').hasProperty('type', 'password'); | ||
| * | ||
| * @see {@link #doesNotHaveProperty} | ||
| */ | ||
| hasProperty(name: string, value: string | RegExp, message?: string): DOMAssertions { | ||
| hasProperty(name: string, value: unknown, message?: string): DOMAssertions { |
There was a problem hiding this comment.
why are you using unknown here instead of RegExp | any as specified above?
There was a problem hiding this comment.
RegExp | any doesn’t make any sense in typescript, it will still evaluate as any. And unknown is safer then any because it doesn’t imply any knowledge about type.
As for JSDoc annotation there’s no unknown type, so any is the closest alternative. And it’s still helpful to mention RegExp because it has special treatment (match instead of just equality check).
There was a problem hiding this comment.
@wuron is correct: types in TS are sets, and since any is already a superset of everything else, having T | any, for any type, is always exactly identical to just having any.
There was a problem hiding this comment.
Addendum: you can actually use unknown in JSDoc and most tools will just show "unknown" there, and anything TS-aware will actually correctly interpret it.
There was a problem hiding this comment.
you can actually use unknown in JSDoc and most tools will just show "unknown" there, and anything TS-aware will actually correctly interpret it.
If that’s preferred I can set it to RegExp | unknown in JSDoc. But does it really change anything? JSDoc definitions are only used as documentation: to generate API.md file, or to hint types for non-TS environments. In both cases we want to communicate that this method accepts any value, hence any works just fine.
In TS definitions we shouldn’t use any, because it’s compatible with all types and allows us to do something like this without any warnings:
function hasProperty(param: any) {
param.callRandomMethod();
const a: boolean = param;
const b: string = param;
}Parameter with type unknown is much safer, and it still allows to pass any value.
Turbo87
changed the title
hasProperty assertion.hasProperty assertion
|
@Turbo87 would you mind cutting a release which includes this? 🙏 Would love to take advantage of it in a small library I'm open-sourcing! |
|
v1.5.0 should be out in a couple of minutes once CI is done :) |
|
Thank you! Really appreciate it! |
Fixes #781