Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Types: return type of zero argument attr() function missing undefined #2315

Closed
NoxHarmonium opened this issue Jan 19, 2022 · 1 comment · Fixed by #2505
Closed

Types: return type of zero argument attr() function missing undefined #2315

NoxHarmonium opened this issue Jan 19, 2022 · 1 comment · Fixed by #2505

Comments

@NoxHarmonium
Copy link

Version: v1.0.0-rc.10

The type definition for attr (without arguments) is the following:

/**
 * Method for getting all attributes and their values of the first element in
 * the matched set.
 *
 * @category Attributes
 * @example
 *
 * ```js
 * $('ul').attr();
 * //=> { id: 'fruits' }
 * ```
 *
 * @returns The attribute's values.
 * @see {@link https://api.jquery.com/attr/}
 */
export declare function attr<T extends Node>(this: Cheerio<T>): Record<string, string>;

However, attr can also return undefined if called on a missing element. For example:

Welcome to Node.js v12.22.3.
Type ".help" for more information.
> const cheerio = require('cheerio')
undefined
> const $ = cheerio.load("<body></body>", { xml: true }, false)
undefined
> $("meta").attr()
undefined

This is unsafe and can cause unexpected runtime errors. The type signature should probably be:

export declare function attr<T extends Node>(this: Cheerio<T>): Record<string, string> | undefined;

I'll see if I can have a dig and find out why the types are generated that way.

Thanks!
fb55 added a commit that referenced this issue Apr 30, 2022
@fb55
fix(types): Add missing undefined return value
0bb0bbe 
Fixes #2315
@fb55 fb55 mentioned this issue Apr 30, 2022
Merged
@fb55
Copy link
Member

fb55 commented Apr 30, 2022

Thanks for the report! This has been fixed in #2505

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix(types): Add missing undefined return value cheeriojs/cheerio
2 participants
@fb55 @NoxHarmonium