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.

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

cheerio@1.0.0-rc12 has itself type files, drop @type/cheerio dependency please #35

Closed
yunsii opened this issue Nov 16, 2023 · 12 comments
Closed

Comments

@yunsii
Copy link
Contributor

yunsii commented Nov 16, 2023

ref: https://unpkg.com/browse/cheerio@1.0.0-rc.12/lib/cheerio.d.ts

I can make a PR if possible.

@cyberalien
Copy link
Member

They don't seem to be usable.

I don't see types for cheerio elements, such as Element and TagElement defined in @types/cheerio.

Then function itself, which has type Cheerio in types, is a mixin in that file, I'm not sure what <T> is supposed to be there.

@yunsii
Copy link
Contributor Author

yunsii commented Nov 16, 2023

I used cheerio@1.0.0-rc12 in my project. After I install @iconify/tools, my written script encounter ts error, because my scripts depends on cheerio@1.0.0-rc12 types, but ts load types from @types/cheerio.

I don't see types for cheerio elements, such as Element and TagElement defined in @types/cheerio.

Then function itself, which has type Cheerio in types, is a mixin in that file, I'm not sure what is supposed to be there.

I will try to test it recently.

@cyberalien
Copy link
Member

What error message are you getting?

@yunsii
Copy link
Contributor Author

yunsii commented Nov 16, 2023

Failed to compile.

../path/to/heading-elements.ts:20:34
Type error: Property 'tagName' does not exist on type 'Element'.
  Property 'tagName' does not exist on type 'TextElement'.

  18 |         const text = $(item).text()?.trim()
  19 |         if (typeof text === 'string' && !text) {
> 20 |           messages.push(`<${item.tagName.toLowerCase()}> textContent is empty`)
     |                                  ^
  21 |         }
  22 | 
  23 |         if (index === 0 && item.tagName.toLowerCase() !== 'h1') {

With this error, I have to use ignoreBuildErrors of Next.js.

@cyberalien
Copy link
Member

Oh, that's a different problem. Type should be TagElement. Probably an error in tools.

@cyberalien
Copy link
Member

I've checked code in tools, it correctly uses TagElement.

If item in code above is not returned by tools, but is retrieved in other way, error is correct. item can be tag, text or comment, but tagName exists only on tag. So you need to check if item.type === 'tag'

@yunsii
Copy link
Contributor Author

yunsii commented Nov 16, 2023

Sorry, it's my cheerio script to parse html, there is no error before install @iconify/tools 😂

@yunsii
Copy link
Contributor Author

yunsii commented Nov 18, 2023

@cyberalien After investigation and try, I create a PR #36 for this, It passed the build:tools and test:tools scripts.

@cyberalien
Copy link
Member

Thanks!

@yunsii
Copy link
Contributor Author

yunsii commented Nov 23, 2023

@cyberalien Is there any plan to release it?

@cyberalien
Copy link
Member

Sure, but pull request is marked as draft. Is it ready?

@yunsii
Copy link
Contributor Author

yunsii commented Nov 23, 2023

Sure, but pull request is marked as draft. Is it ready?

I was waiting for you code review before, if no more comments, it's ready now 😂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants