Implement for...of iterator via Symbol.iterator #1197

papandreou · 2018-05-12T23:32:26Z

Similar to jQuery: https://github.com/jquery/jquery/blob/1ea092a54b00aa4d902f4e22ada3854d195d4a18/src/core.js#L371-L373

Fixes #1191

Similar to jQuery: https://github.com/jquery/jquery/blob/1ea092a54b00aa4d902f4e22ada3854d195d4a18/src/core.js#L371-L373 Fixes cheeriojs#1191

fb55 · 2018-05-13T15:00:31Z

This is a great way to implement this. Changed the base to the v1.0 branch, will merge this once the tests pass.

jugglinmike

I like the approach--glad to see you've done your homework. Just a couple changes and we should be good to go

jugglinmike · 2018-05-13T17:08:47Z

lib/cheerio.js

@@ -138,6 +139,11 @@ Cheerio.prototype.toArray = function() {
  return this.get();
 };

+// Support for (const element of $(...)) iteration:
+if (typeof Symbol !== 'undefined') {
+  Cheerio.prototype[Symbol.iterator] = Array.prototype[Symbol.iterator];


This will fit better in api/traversing.js

That was what I tried first (so it could live near each), but it didn't work because the export of api/traversing.js is being mixed into Cheerio.prototype using _.extend, which does not include Symbol keys:

const mySymbol = Symbol('foo'); const a = { [mySymbol]: 123 }; console.log(a[mySymbol]); // 123 const b = require('lodash').extend({}, a); console.log(b[mySymbol]); // undefined

Fixing that would involve iterating over Object.getOwnPropertySymbols(mod) as well, and that seemed way too invasive.

jugglinmike · 2018-05-13T17:10:35Z

test/api/traversing.js

+            iterator = $li[Symbol.iterator]();
+        expect(iterator.next().value.attribs['class']).to.equal('apple');
+        expect(iterator.next().value.attribs['class']).to.equal('orange');
+        expect(iterator.next().value.attribs['class']).to.equal('pear');


Can you also assert that iteration terminates as expected?

Absolutely, fixed in c6bdc79

cheeriojs#1197 (comment)

fb55 · 2018-05-14T10:48:33Z

Awesome, thanks a lot!

* Use parse5 as a default parser (closes #863) * Use documents via $.load * Add test for #997 * Change options format * Update unit test Update test phrasing according to recent changes in parsing logic. * 1.0.0-rc.1 * Improve release process Limit responsibility of "pre-publish" script to simply validate the project's `History.md` file (by verifying an entry for the current release). Define a separate script for history generation. Separating the workflow in this way allows for manual modification of the release notes. * Correct errors in Readme.md * Document advanced usage with htmlparser2 * Update History.md (and include migration guide) * Remove documentation for `xmlMode` option Simply expose an option named `xml` that enables XML parsing via htmlparser2 with the ability to specify additional options for that parser. * Rename `useHtmlParser2` option This flag is used to control parsing behavior internally, but it is not intended for use by consumers. Prefix the name with an underscore in order to discourage users from relying on it. * Re-write migration guide for version 1.0 (#1078) Incorporate recent feedback from consumers who have experimented with the version 1.0 release candidate. * Pass locationInfo option to parse5 (#1155) * Update css-select to the latest version 🚀 (#1158) Breaking change: Invalid selectors now throw Errors, not SyntaxErrors. * Use eslint & prettier, add precommit hook (#1152) * chore(package): update mocha to version 5.0.4 (#1088) * Ensure text nodes expose the DOM level 1 API Since enabling the `withDomLvl1` parsing option, nodes cannot be created with an object literal. Create new text nodes using the `evaluate` function to ensure they expose the correct attributes. * fixing missing prop(‘outerHTML’) implementation. Added an ‘outerHTML’ case to the switch in the prop function, which wraps a clone of in a container element, and sets to that container's HTML (#945) * Do not lint files excluded from version control (#1162) This includes code coverage reports as generated by the command `make test-cov`. * Correct typo in git hook configuration (#1163) * Correct typo in git hook configuration * Reformat package manifest to satisfy linter * Fix .text with a function as the argument * Fix `.text` being called on a collection with size > 1 with a function * chore(package): update coveralls to version 3.0.0 (#1086) * Update jsdom to the latest version 🚀 (#1008) * Throw a useful error on invalid input to cheerio.load() (#1087) * Procedurally generate API documentation from source (#1165) * Use parse5 to serialize the DOM, use lodash to clone dom * Fix DoS/RCE vulnerability in lodash@4.15.0 (#1179) fixes #1175 * Add eslint-plugin-jsdoc, improve documentation (#1168) * Improve variable names (#1183) Promote consistency in variable names within the project's source and unit tests. This helps to highlight the distinction between the object exported by the module and the function produced by the `load` method. The latter value is expected to mimic the jQuery API, while the former value generally should only be used for a small set of methods specific to Cheerio: - `load` - `html` - `xml` - `text` Other usages of the exported object are discouraged, and a future patch will update the unit tests to reflect the usages that are endorsed for long-term stability. * Formally test deprecated APIs (#1184) Methods named `load`, `html`, `xml`, and `text` are defined in many locations: Today, Cheerio defines multiple versions of methods named `load`, `html`, `xml`, `text`, and `parseHTML`. These alternate versions may be defined in up to three distinct parts of the API: - exported by the Cheerio module - as static methods of the "loaded" Cheerio factory function - as instance methods of the "loaded" Cheerio factory function Some of these are surperfluous, and because some unecessarily conflict with idiomatic jQuery coding patterns, they have been designated for future removal [1]. Add tests for the deprecated methods in order to avoid regressions prior to their removal. Insert comments to delineate the methods which are endorsed and which have been deprecated. For the latter group of methods, include recommendation for the preferred alternatives. [1] #1122 * Implement for...of iterator via Symbol.iterator (#1197) * Implement for...of iterator via Symbol.iterator Similar to jQuery: https://github.com/jquery/jquery/blob/1ea092a54b00aa4d902f4e22ada3854d195d4a18/src/core.js#L371-L373 Fixes #1191 * Assert that the iterator ends #1197 (comment)

* Implement for...of iterator via Symbol.iterator Similar to jQuery: https://github.com/jquery/jquery/blob/1ea092a54b00aa4d902f4e22ada3854d195d4a18/src/core.js#L371-L373 Fixes #1191 * Assert that the iterator ends #1197 (comment)

Implement for...of iterator via Symbol.iterator

d8aaef5

Similar to jQuery: https://github.com/jquery/jquery/blob/1ea092a54b00aa4d902f4e22ada3854d195d4a18/src/core.js#L371-L373 Fixes cheeriojs#1191

papandreou mentioned this pull request May 12, 2018

Support for...of and entries iteration? #1191

Closed

fb55 changed the base branch from master to v1.0.0 May 13, 2018 14:59

Merge branch 'v1.0.0' into feature/iteratorSymbol

4f27a2d

jugglinmike reviewed May 13, 2018

View reviewed changes

Assert that the iterator ends

c6bdc79

cheeriojs#1197 (comment)

fb55 merged commit f698a31 into cheeriojs:v1.0.0 May 14, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement for...of iterator via Symbol.iterator #1197

Implement for...of iterator via Symbol.iterator #1197

papandreou commented May 12, 2018

fb55 commented May 13, 2018

jugglinmike left a comment

jugglinmike May 13, 2018

papandreou May 13, 2018 •

edited

Loading

jugglinmike May 13, 2018

papandreou May 13, 2018 •

edited

Loading

fb55 commented May 14, 2018

Implement for...of iterator via Symbol.iterator #1197

Implement for...of iterator via Symbol.iterator #1197

Conversation

papandreou commented May 12, 2018

fb55 commented May 13, 2018

jugglinmike left a comment

Choose a reason for hiding this comment

jugglinmike May 13, 2018

Choose a reason for hiding this comment

papandreou May 13, 2018 • edited Loading

Choose a reason for hiding this comment

jugglinmike May 13, 2018

Choose a reason for hiding this comment

papandreou May 13, 2018 • edited Loading

Choose a reason for hiding this comment

fb55 commented May 14, 2018

papandreou May 13, 2018 •

edited

Loading

papandreou May 13, 2018 •

edited

Loading