Normalizing the public API

[jugglinmike]

The project has two notions of "static methods":

1. Methods defined on the Cheerio constructor function. There are six in total:
   • 3 mimic methods defined on the global function defined by the jQuery project: .contains(), .merge(), and .parseHTML()
   • 3 are specific to Cheerio, exposed to address concerns that are not relevant in the browser context .html() and .xml(), .root()
2. Methods defined on the module's exported value (e.g. the value returned from require('cheerio')). There are two of these: .load() and .text().

As of today, however, the source does not reflect this distinction. All eight methods are defined in both places.

The exported value is itself the constructor for the Cheerio object. We have never documented the direct use of this constructor. As far as I remember, we have always instructed folks to use it indirectly, after being "bound" by the load method (as trying to use it directly would involve re-implementing the load method).

These details make the API difficult to document, and they may cause confusion for developers who are new to the project. (The methods in the first set don't make sense in the context of the Cheerio module itself, and the methods in the second set don't apply to a Cheerio function that has been "bound" to a document.) Because we are preparing for a major release, I would like to discuss taking the opportunity to make a breaking change:

1. Export an ordinary object value (not a function)
2. Re-implement exports.html() and exports.xml() to operate in this context (rendering the "outer" markup of a "selection" defined by a provided Cheerio object)
3. Persist the following exported values: exports.load(), exports.text(), and exports.version
4. Remove the Cheerio.load() and Cheerio.text() methods from the Cheerio constructor function

#2 directly contradicts the documentation on rendering, but the upgrade path is fairly straightforward:

• $.html('.pear') becomes cheerio.html($('.pear'))
• $.html($('.pear')) becomes cheerio.html($('.pear'))
• $.html() becomes cheerio.html($.root())

I think this is warranted because it makes the API easier to understand/document and because it further reduces disparities between "Cheerio objects" and "jQuery objects".

@fb55 @matthewmueller does this sound good to you? And are there any consumers out there that can share a use case which would be prevented by the change I'm proposing?

[fb55]

Sorry for the delay, was traveling. This sounds exactly like the kind of change we should consider for the final 1.0. This should be a nice simplification, and remove some pitfalls — I'm all for it.

[matthewmueller]

Would it be possible to upgrade this without breaking anything? I agree with these changes and given that we're still in 0.x, I'm open to this change.

I do think it'd be better if we can avoid breaking anything, alias these functions and update the docs with the improved API.

Lots of users these days – over 5 million downloads per month (!!). For better or for worse, there's going to be some package.json's with { "cheerio": "*" } 🙃

[jugglinmike]

Thanks for the review, you two!

Would it be possible to upgrade this without breaking anything? I agree with
these changes and given that we're still in 0.x, I'm open to making this
change.

I do think it'd be better if we can avoid breaking anything and alias these
functions. lots of users these days – over 5 million downloads per month (!!)

One way to think about the current API is that it already defines a bunch of
aliases. For instance, $.html() is just a convenience for
cheerio.html($.root()). Removing the aliases (i.e. making breaking changes)
is the goal.

The change we're discussing is unrelated to correctness, so there's a case to
be made for not touching anything. In other words, "If it ain't broke, don't
fix it."

From my perspective, though, it is broken. The convenience APIs are not available
in jQuery. It's also generally harder to document and learn about an API that
offers many ways to do the same thing.

I should mention that I made an error in the description of this issue. When I
wrote,

The exported value is itself the constructor for the Cheerio object. We have
never documented the direct use of this constructor. As far as I remember, we
have always instructed folks to use it indirectly, after being "bound" by the
load method (as trying to use it directly would involve re-implementing the
load method).

I was wrong. The README.md file contains the following examples:

Optionally, you can also load in the HTML by passing the string as the context:

const $ = require('cheerio');
$('ul', '<ul id="fruits">...</ul>');

Or as the root:

const $ = require('cheerio');
$('li', 'ul', '<ul id="fruits">...</ul>');

This doesn't make the change any more "breaking", but it probably means that
more consumers rely on the API than my original report suggested.

If we'd like to be more conservative, we could release 1.0 without this change,
and instead document the methods as "deprecated". In that case, I would like to
also announce an estimated release date for version 2.0 where we eventually do
remove those things. But I'm not sure this is actually helpful--I think most
consumers will be unaware of API deprecation status, so deferring the change
just delays the interruption. We're already planning to break some use cases
for the 1.0 release. If we're all in agreement that this change is desirable,
then it seems better to get this change over with.

[fb55]

I always found the constructor pretty confusing and would agree with deprecating its direct usage. Removing it directly might be a bit too aggressive, so for now just adding a deprecation notice if using any of the methods we want to remove is IMHO the best way forward.

[jugglinmike]

I agree that it's in everyone's interest if we don't move quite so fast on this--I just needed some time to slow down :)

Still, the 1.0 announcement will be a great time to alert users to our intention here, and I think there are some things we can do to make the transition easier.

To move toward this goal for version 1.0, I'd like to document the irregular API as "deprecated" but also to improve test coverage. Currently, our coverage is incomplete, and that's particularly hard to see given the irregular use of the $ internally.

1. Improving variable names used within the test suite to consistently refer to the module value as cheerio and the "loaded" factory function as $
2. Extending test coverage to include the deprecated APIs
3. Refactoring the internals to more clearly designate the deprecated APIs

We just landed a patch for step 1, so patches for the next steps should be easier to read. A patch for step 2 is currently under review. Once that's through, we'll have reasonable certainty that the refactoring in step 3 isn't introducing any regressions in those unfortunate methods.

[matthewmueller]

From my perspective, though, it is broken. The convenience APIs are not available
in jQuery. It's also generally harder to document and learn about an API that
offers many ways to do the same thing.

Ah I think I got it, so you're coming at this as "let's align ourselves better with jQuery". That makes a lot of sense to me.

I'm happy to accept whatever you decide here, but my preference would be that we undocument those irregularities, while leaving them in and perhaps adding a deprecation note. However, in any case where we're at odds with jQuery, that'd be a really good signal that we should make a breaking change.

Everything else also sounds really wonderful. The $(...) stuff is definitely some hairy code.

Thanks for taking the charge on this!

[jugglinmike]

I'm happy to accept whatever you decide here, but my preference would be that we undocument those irregularities, while > leaving them in and perhaps adding a deprecation note.

I know where you're coming from--it's why I initially suggested we remove the code outright. By deciding to deprecate for 1.0 (rathen than remove, as would be our right in a major release) then we're recognizing that some users are currently dependent on that code and that we don't want to shut them out. Removing documentation will make things harder for them, but adding documentation with clear deprecation notices more closely aligns with our committments. It may also cause some folks to discover and use the methods, but I'll have less concern about breaking their code in Cheerio version 2

Everything else also sounds really wonderful. The $(...) stuff is > definitely some hairy code.

Thanks for taking the charge on this!

Awesome, thanks for saying so!

[jugglinmike]

We just completed step 3 via gh-1232, so it's safe to call this issue resolved :)