[Deps] [breaking] update `cheerio` to v1 #1093

ljharb · 2017-08-28T05:26:30Z

See cheeriojs/cheerio#1047 (comment) for context.

Hoping to merge this ASAP so it can be part of v3.

This semver range also will include ^1.0.0 once it's released, verified on https://semver.npmjs.com/

aweary · 2017-08-28T15:02:02Z

@ljharb test suite seems to be failing

TypeError: _cheerio2.default.load(...)(...).root is not a function

ljharb · 2017-08-28T15:11:56Z

Yes, I can't get them running locally yet to fix :-/

jugglinmike · 2017-08-29T00:02:11Z

root is a static method on the jQuery-like object produced by load. I guess
the recommendation we made is a little inaccurate. I'll think about how to make
a better explanation for our migration guide.

For your purposes here, could you try exposing simply cheerio.load('')(html)?

ljharb · 2017-08-29T00:24:52Z

What's the functional difference? Is the fragment not jquery-like?

What about cheerio(cheerio.load('')(html))?

jugglinmike · 2017-08-29T14:01:47Z

Here again, my explanation is more confusing than helpful. "jQuery-like" can
mean a couple things, depending on your interpretation of "jQuery".

The return value of cheerio.load('') could be called "jQuery-like" because it
returns a function that is like the global function defined by jQuery (i.e.
window.$ or window.jQuery). This functions has the static methods defined
by jQuery such as parseHTML, but it also has a Cheerio-specific static method
named root.

The return value of cheerio.load('')('<div>') is "jQuery-like" because it
returns an array-like object whose members are elements (sometimes called a
"jQuery collection").

My apologies for making this more confusing than it needs to be. If it's any
consolation, this conversation is going to help me improve Cheerio's migration
guide, and that will hopefully make the transition easier for others when we do
publish version 1.0. I'd be happy to contribute to this branch and get things
passing, but I may not have any availability until this weekend. Please let me
know!

ljharb · 2017-08-29T15:35:37Z

That sounds great; I'll add you to my fork today so you can add to this PR.

nfcampos · 2017-08-29T18:30:08Z

.travis.yml

@@ -3,12 +3,12 @@ node_js:
  - "8"
  - "6"
  - "4"
+before_install:
+  - 'if [ "${TRAVIS_NODE_VERSION}" = "0.6" ]; then npm install -g [email protected] ; elif [ "${TRAVIS_NODE_VERSION}" != "0.9" ]; then case "$(npm --version)" in 1.*) npm install -g [email protected] ;; 2.*) npm install -g npm@2 ;; esac ; fi'


why do we need this and the install script?

It ensures npm doesn't break, and deps can be installed on any node version. This will be rebased out tho; only the latest commit on the PR matters.

but we dont use versions of node in travis other than 8, 6 and 4, so why test for 0.6 and 0.4, etc?

Because then I can copy paste my known-correct Travis config lines from all my other repos :-)

nfcampos · 2017-08-29T18:36:24Z

.travis.yml

@@ -19,7 +19,7 @@ sudo: false
 matrix:
  fast_finish: true
  include:
-   - node_js: "6"
+   - node_js: "node"


why this change?

There's no benefit to pinning the linter on node 6.

is there a benefit to pinning the karma tests, seeing as they run in a browser?

Only if the karma tools are c deps. I don't know much about karma.

i dont think it makesany difference, but no point in changing it now

The effective breaking change is that now `render` returns a wrapper around the *rendered thing*, like shallow and mount - instead of a “root” that requires you dive into `.children()` See cheeriojs/cheerio#1047 (comment)

ljharb · 2017-08-30T01:07:40Z

OK, got this working :-)

The effective breaking change is that now render returns a wrapper around the rendered thing, just like shallow and mount will (once v3 is done) - instead of a “root” that requires you dive into .children(). This will make all our APIs more consistent.

jugglinmike · 2017-08-30T23:37:35Z

I discovered the same thing last night, but I didn't have the time/imagination
to think of a workaround to preserve Enzyme's current behavior. Even now, I'm
not sure it's possible--that old root "element" was a little like a document
fragment in that it had children but no representation in the rendered DOM.

...so I'm glad that not only is the breaking change acceptable, but that you
find it preferable for consistency's sake!

On the Cheerio side, I plan on updating the migration guide with information
about the root method. We didn't modify its implementation, but now that
we're working with more realistic documents, users should be aware that what
they get back will be different than prior releases (though more in-line with
the browser overall).

nfcampos

LGTM

nfcampos · 2017-08-31T18:21:40Z

@ljharb are we compiling a list of breaking changes for v3?

ljharb · 2017-09-01T00:08:29Z

@nfcampos i believe leland has one in progress.

ljharb mentioned this pull request Aug 28, 2017

Add 1.0.0-rc.1 to History.md cheeriojs/cheerio#1047

Closed

ljharb added semver: major Breaking changes API: render labels Aug 28, 2017

ljharb requested review from nfcampos, lelandrichardson, blainekasten, aweary and a team and removed request for nfcampos, blainekasten and aweary August 28, 2017 05:27

ljharb force-pushed the cheerio branch from e7afc94 to 6313269 Compare August 29, 2017 07:33

ljharb force-pushed the cheerio branch from 6313269 to f380817 Compare August 29, 2017 16:52

nfcampos reviewed Aug 29, 2017

View reviewed changes

ljharb force-pushed the cheerio branch from f380817 to 41d007d Compare August 29, 2017 21:53

ljharb added 2 commits August 29, 2017 16:07

[Tests] add more render tests

4b766af

[Deps] [breaking] update cheerio to v1

f7c5a15

The effective breaking change is that now `render` returns a wrapper around the *rendered thing*, like shallow and mount - instead of a “root” that requires you dive into `.children()` See cheeriojs/cheerio#1047 (comment)

ljharb force-pushed the cheerio branch 2 times, most recently from 79f47ee to d6918c4 Compare August 30, 2017 00:56

ljharb added the v3 blocker label Aug 30, 2017

ljharb force-pushed the cheerio branch from d6918c4 to 612473d Compare August 30, 2017 21:47

nfcampos approved these changes Aug 31, 2017

View reviewed changes

ljharb force-pushed the cheerio branch 4 times, most recently from 8339ced to f7c5a15 Compare September 1, 2017 08:07

ljharb merged commit 27d05a0 into enzymejs:master Sep 5, 2017

ljharb deleted the cheerio branch September 5, 2017 19:51

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Deps] [breaking] update `cheerio` to v1 #1093

[Deps] [breaking] update `cheerio` to v1 #1093

ljharb commented Aug 28, 2017 •

edited

Loading

aweary commented Aug 28, 2017

ljharb commented Aug 28, 2017

jugglinmike commented Aug 29, 2017

ljharb commented Aug 29, 2017

jugglinmike commented Aug 29, 2017

ljharb commented Aug 29, 2017

nfcampos Aug 29, 2017

ljharb Aug 29, 2017

nfcampos Aug 29, 2017

ljharb Aug 29, 2017

nfcampos Aug 29, 2017

ljharb Aug 29, 2017

nfcampos Aug 29, 2017

ljharb Aug 29, 2017

nfcampos Aug 31, 2017

ljharb commented Aug 30, 2017

jugglinmike commented Aug 30, 2017

nfcampos left a comment

nfcampos commented Aug 31, 2017

ljharb commented Sep 1, 2017

[Deps] [breaking] update cheerio to v1 #1093

[Deps] [breaking] update cheerio to v1 #1093

Conversation

ljharb commented Aug 28, 2017 • edited Loading

aweary commented Aug 28, 2017

ljharb commented Aug 28, 2017

jugglinmike commented Aug 29, 2017

ljharb commented Aug 29, 2017

jugglinmike commented Aug 29, 2017

ljharb commented Aug 29, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ljharb commented Aug 30, 2017

jugglinmike commented Aug 30, 2017

nfcampos left a comment

Choose a reason for hiding this comment

nfcampos commented Aug 31, 2017

ljharb commented Sep 1, 2017

[Deps] [breaking] update `cheerio` to v1 #1093

[Deps] [breaking] update `cheerio` to v1 #1093

ljharb commented Aug 28, 2017 •

edited

Loading