node-htmlparser icon indicating copy to clipboard operation
node-htmlparser copied to clipboard

Support for RequireJS

Open bitter opened this issue 15 years ago • 7 comments

Hi!

I made some changes which makes the htmlparser compatible with RequireJS and I also added two test runners for that environment. All I had to do was remove the detection of '__filename' and '__dirname'. Those global variables are node specific and not part of CommonJS.

I did not do a new minified version because I don't know which minification tool you use.

Anyhow, pull the change if you like it. If not, no biggy.

bitter avatar Oct 11 '10 18:10 bitter

Will pull changes for 1.8

tautologistics avatar Dec 20 '10 14:12 tautologistics

Great! Thanks for the htmlparser. It's much faster than the browser when you need to traverse html, the stupid browser always seem to load images and other bogus resources.

bitter avatar Jan 07 '11 02:01 bitter

1.8 is coming together finally. I merged your changes into a bitter-master branch (https://github.com/tautologistics/node-htmlparser/tree/bitter-master) but I am having some issues getting tests to pass:

Total tests: 22
Failed tests: 0

node.js:116
    throw e; // process.nextTick error, or 'error' event on first tick
    ^
TypeError: Cannot read property 'type' of null
at runtests.rjs.js:39:36
at Function.execCb (require.js:1528:25)
at execManager (require.js:505:27)
at Object.onDep (require.js:579:33)
at execManager (require.js:536:45)
at main (require.js:655:17)
at callDefMain (require.js:668:18)
at Object.completeLoad (require.js:1181:21)
at Function.load (node.js:75:17)
at loadPaused (require.js:916:21)

One minor problem is that the summary (Total tests/Failed tests) is printed right away instead of after the async tests are run. The main issue is that the inner require() that loads the tests is passing a null module parameter to the callback. I do not have enough experience with RequireJS or the node shim to dig into this quicklu. Do you have time to ake a look?

tautologistics avatar Mar 06 '11 19:03 tautologistics

Absolutely, can I ask which version of node you are using? When I wrote the code I was using version 0.2.5 which would be considered quite old now-a-days :-)

bitter avatar Mar 07 '11 10:03 bitter

Hmm... I just realized that running the tests using node is not such a good idea since the only thing they prove is that the tests can be run using a requirejs adapter. I would like to rewrite the test to be a browser test instead, that way the htmlparser (and the tests) will be loaded using requirejs.

bitter avatar Mar 07 '11 11:03 bitter

It looks like a lot has changed since I created my patch and I'm not sure the patch is valid anymore. I'll dig some more but as of now I would recommend that you drop the RequireJS compatibility, I'll try to make a new patch when I figure out the state of the RequireJS node adapter.

bitter avatar Mar 07 '11 12:03 bitter

I have been working against node v0.4.1 and started with your version of r.js. Then that failed I pulled the latest from requirejs.org and still got the same thing. Unless there is a big problem, there can still be a requirejs version of the runtests.js in addition to the runtests.html; the more coverage the better.

For now the changes will remain in the bitter-master branch until you have an update or I have some time.

tautologistics avatar Mar 07 '11 14:03 tautologistics