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.

Sign up for GitHub

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

Jump to bottom

Update jsdom in htmlizer 0.x #21

Closed
mwoc opened this issue Jun 14, 2016 · 7 comments
Closed

Update jsdom in htmlizer 0.x #21

mwoc opened this issue Jun 14, 2016 · 7 comments

Comments

@mwoc
Copy link
Contributor

mwoc commented Jun 14, 2016

We're using htmlizer 0.8.0 in a project we wish to update to node v6, without updating to htmlizer v2 in the same iteration. This breaks due to jsdom's interface supposedly breaking when recompiling on nodejs v6:

___/htmlizer/node_modules/jsdom/lib/jsdom/level1/core.js:553
Array.prototype.splice.call(this._childNodes, refChildIndex, 0, newChild);
^

TypeError: Cannot set property length of [object Object] which has only a getter
at DocumentFragment.core.Node.insertBefore (/htmlizer/node_modules/jsdom/lib/jsdom/level1/core.js:553:30)
at DocumentFragment. (/htmlizer/node_modules/jsdom/lib/jsdom/level2/events.js:332:20)
at DocumentFragment.proto.(anonymous function) as insertBefore
at DocumentFragment.core.Node.appendChild (_/htmlizer/node_modules/jsdom/lib/jsdom/level1/core.js:671:17)
at ___/htmlizer/src/jquery.js:3:2360
at __/htmlizer/src/jquery.js:3:2615
at b.object.module.object.module.exports.module.exports (/htmlizer/src/jquery.js:2:201)
at window (_/htmlizer/src/Htmlizer.js:15:32)
at Object. (_/htmlizer/src/Htmlizer.js:24:2)

I tested with small increments to the jsdom version, and found that updating from 0.10.3 to 1.5.0 seems enough. For now, we've made a fork at https://github.com/mwoc/htmlizer/tree/v0 with this update, which probably is ok - but if others might need htmlizer v0.x to work on nodejs v6, then maybe an official patch might be worthwhile.
@Munawwar
Copy link
Owner

Is it ok if I update to jsdom 9.2.1?

Munawwar added a commit that referenced this issue Jun 18, 2016
@Munawwar
1. Updated jsdom 2. DOCTYPE detection. (See full commit message). #21
bca1680 
1. jQuery.parseHTML on jsdom 3.1.2+ removes doctype, html, head and body tag. As a workaround,
some handling for full HTML document template is done based on an assumption that the first tag
of the document is an <html> tag (excluding doctype) when one wants to parse an entire document.

2. Detects doctype and prepend it to toString() output.
.
@Munawwar
Copy link
Owner

Munawwar commented Jun 18, 2016 •
edited
Loading

Ok, now I remember why I didn't update jsdom.

Htmlizer relies on jQuery.parseHTML for parsing HTML string to DOM nodes.
Upgrading to jsdom 3.1.2 or higher makes jQuery.parseHTML removes doctype,html,body and head tags. On jsdom 0.10.3 jQuery.parseHTML removes doctype only. Upgrading jsdom would cause a regression for anyone parsing entire documents (like here).

As a workaround, now I detect if document begins with a <html> tag and then use a different parsing trick. The limitation are:

  1. you can't parse a template that begins with a head or body tag. They will have to be wrapped inside an html tag.
  2. An empty head tag will always be present when parsing an entire document (I can work around that as well, but didn't, as it won't cause much harm I guess).

Tell me if you are ok with the above limitations.

@papandreou
Copy link
Contributor

@mwoc, upgrading to htmlizer 2 should be far easier. It doesn't depend on jsdom at all (except as a dev dep), is faster and has the same API (iirc).

@Munawwar
Copy link
Owner

Munawwar commented Jun 19, 2016 •
edited
Loading

@mwoc Yea, very few things have changed in htmlizer2. From the README file:

1. toDocumentFragment() method has been removed.
2. $element KO binding context cannot be used anymore.
3. template binding interface has changed. (It has become simpler).

BTW, has anyone noticed that knockout itself can be run on node (by first declaring a global window and document object)? As far as I've tested it, even ko.observable and change updates also works.

@mwoc
Copy link
Contributor Author

mwoc commented Jun 19, 2016

I'll take a look at upgrading to v2 then. I think I tried while I was upgrading to node v6, and something else broke when using htmlizer, probably hitting one of the API changes. Will let you know how it goes this time :)

@Munawwar
Copy link
Owner

@mwoc Did you solve your issues with v2?

@mwoc
Copy link
Contributor Author

mwoc commented Aug 18, 2016

I'm working now on updating to v2, and ran into issue #22 which I now remember earlier was the reason why I wanted to stick to v0.x for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@papandreou @Munawwar @mwoc