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

Initial browserify support #332

Closed

Conversation

kirbysayshi
Copy link

Initial stab at #268.

Depends on fb55/entities#12, as well as fb55/css-select#16.

Simple fix to get around the conditional require for lib-cov.
@fb55
Copy link
Member

fb55 commented Dec 19, 2013

Maybe the version should also be added to the regular package. So far, I don't see any real reasons for maintaing a separate file for it.

@jugglinmike
Copy link
Member

My thought's exactly. Also: we shouldn't define a separate build until we have tests for it.

@kirbysayshi
Copy link
Author

@fb55 What do you mean by "added to the regular package"? Browserify has trouble with the conditional require of lib-cov.

@jugglinmike I agree. I tried adding a test similar to

var browserify = require('browserify');
var vm = require('vm');
var path = require('path');

var b = browserify();
b.require('../', { expose: 'cheerio' });
b.bundle(function(err, src) {
  console.log(err);

  var ctx = { console: console };

  vm.runInNewContext(src, ctx);
  console.log(ctx.require('cheerio'))
});

But browserify was having a really hard time loading the current module.

@jugglinmike
Copy link
Member

But browserify was having a really hard time loading the current module.

I don't understand. By "the current module", do you mean Cheerio itself? And isn't that expected (given that the patches this depends on have not been published yet)?

@kirbysayshi
Copy link
Author

Yes, cheerio itself. Ideally it would test that it could browserify and execute some dummy cheerio code.

I had symlinked in packages that compensated for the patches not having landed, and browserify was still failing to load cheerio itself. I could have been confused though, I was doing a lot of symlinking and every time I fixed one issue, another one cropped up!

So far the biggest stopper is the one in node-entities where there are requires that map across several different strings.

@jugglinmike
Copy link
Member

I see @kirbysayshi . I appreciate you doing the leg work for this, but at the moment, it doesn't sound like this pull request is suitable to merge. How would you feel about closing this until Cheerio could be built using browserify and that build could be tested?

@ganeshv
Copy link

ganeshv commented Jan 10, 2014

If anyone wants a quick and dirty browserified cheerio: https://gist.github.com/ganeshv/8349757 Haven't tested it all that much, but it's enough to meet my current requirements.

I'm looking forward to the official version. Hopefully there is a way to get a browser-based version somewhat smaller than the current ~450KB. Cheerio/htmlparser2 is exactly what I was looking for - I want the ability to process HTML fragments with a standard JQuery-like API. I can't use JQuery itself because I don't want the side effects of browser-DOM-based parsing (which causes images to be loaded, for instance).

@fb55 fb55 closed this Feb 23, 2014
pjanik added a commit to concord-consortium/lab that referenced this pull request Mar 6, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants