Cheerio: Consider owning @types/cheerio type definitions

Created on 31 Aug 2020  Â·  9Comments  Â·  Source: cheeriojs/cheerio

Problem

In the early days, jQuery familiarity was one of the main attractions to Cheerio. Nowadays it may be more of a hindrence.

For example, it's confusing that Cheerio's $(selector).each(fn) has a different argument order than ES5's forEach.

const fruits = [];

$('li').each(function(i, elem) {
  fruits[i] = $(this).text();
});

fruits.join(', ');
//=> Apple, Orange, Pear

Potential Solution

One way to reduce the friction is to have autocomplete. We currently support some autocomplete via JSDoc but the type definitions would take it a step further.

Some wonderful folks have been maintaining @types/cheerio. Installing this extra package is quite obvious for Typescript users but less obvious for Javascript developers.

We could make this simpler by hosting those types within the Cheerio repository. This would be one less thing to install and the type definitions would get picked up for Javascript users.

Downsides

  • The maintenance burden is now on the Cheerio maintainers.
  • Any other downsides?

Most helpful comment

Fixed in #1491

All 9 comments

Writing a scraper, as I am wont to do at least once per year, and straight up came here to ask why it wasn't hosted in the repo yet.

Timing was good, it seems. :smile:

@AzSiAz Afaict you seem to be the person most on top of the current type definitions (thank you or that!). Any concerns about us pulling the type definitions into the cheerio repo? Anyone else you think we should loop in?

@fb55 Thanks for asking, glad to see you're a considering owning the type definition :smile:

Maybe the other typing maintainer so they know if they need to make modification but I think most of them don't participate anymore so do as you see fit on that front

The only concern coming to mind is to be careful about the node definition included in the current types, you may have to remove/"polyfill" it instead since many people seem to have a problem with it, and it's not really a concern but there is also a PR open here: you might want to check with the author to get change concerning cheerio integrated

Also, don't forget to delete types on definitelyTyped and update types depending on cheerio, might be a pain thought^^

Thanks a lot @AzSiAz, this is very helpful.

@paulmelnikow Should we use your PR as a basis for pulling in types? Any potential downsides that we should expect?

Also cc @blittle @wmaurer @umarniz @LiJinyao @chennakrishna8 @nwtgck @privatenumber — how do you feel about cheerio pulling in the type definition? Any concerns?

Yes, I think it is a great idea to pull in the types :+1:.

Cheerio does have a lot of dependencies and I don’t know how it will work when other DefinitelyTyped packages need to depend on types in cheerio. However I bet DT has a solution for that!

I would suggest, since my PR is making a pretty big change, maybe it’s better to get it merged and shipped from DT (assuming that can be done timely). Then you can pull in something that’s nearly identical to the latest shipped version.

@paulmelnikow thanks for working on this!

Cheerio does have a lot of dependencies and I don’t know how it will work when other DefinitelyTyped packages need to depend on types in cheerio. However I bet DT has a solution for that!

Personally, I think we can copy these type definitions in and let DT sort it out on their end. They might need to do something like:

// inside @types/chai-enzyme
import type { Cheerio } from 'cheerio'

Another thing we'd need is to make sure we have are tests for our type definitions.

@paulmelnikow how does this test work in DT? Does it run assertions?

I've also seen https://github.com/SamVerschueren/tsd used to test types.

@paulmelnikow how does this test work in DT? Does it run assertions?

Yea, it's checking that the provided code compiles successfully, or in the case of $ExpectType that the expression has the type it's supposed to have.

We use tsd to check the type definitions in Shields. Would be a good way to verify the types when they're moved to the Cheerio repo.

I suppose we should hold off on deprecating the DT types until the 1.0.0 release is made.

Fixed in #1491

Was this page helpful?
0 / 5 - 0 ratings

Related issues

collegepinger picture collegepinger  Â·  3Comments

Tetheta picture Tetheta  Â·  3Comments

M3kH picture M3kH  Â·  4Comments

francoisromain picture francoisromain  Â·  5Comments

misner picture misner  Â·  3Comments