WebReflection / basicHTML

A NodeJS based, standard oriented, HTML implementation.
ISC License
126 stars 10 forks source link

Dynamic class attribute values rendered from hyperHTML bind/wire causing `TypeError: Cannot read property 'classList' of null` #12

Closed dsadhanala closed 6 years ago

dsadhanala commented 6 years ago

Not sure if this is basicHTML issue or hyperHTML, but I strongly believe this whole issue was caused by this [line] (https://github.com/WebReflection/hyperHTML/blob/master/esm/objects/Updates.js#L392).

here are the links from runkit for your reference. with basicHTML with JSDOM

moving conversation from #190

Thanks for your time!

WebReflection commented 6 years ago

when in doubt, you can always go in this page https://webreflection.github.io/hyperHTML/test/lib.html and paste relevant code in its console

this looks like basicHTML issue, will have a look, thanks

WebReflection commented 6 years ago

P.S. the JSDOM error is different, it looks like attribute nodes have no cloneNode and it won't be fixed anywhere because JSDOM is not a target

dsadhanala commented 6 years ago

Cool, thanks for looking into.

you can always go in this page https://webreflection.github.io/hyperHTML/test/lib.html

I see a blank page from the URL you have provided above.

P.S. the JSDOM error is different

Sure, I have switched to basicHTML anyway :) and hope I don't get blocked because of the limitations of selectors, I do noticed querySelector is not working as expected, though I was able to workaround with firstElementChild and lastElementChild for now.

do you have any alternative recommendation for selectors? I don't think I can query specific elements with my current approach.

WebReflection commented 6 years ago

I see a blank page from the URL you have provided above.

yes, I'm planning to make hyperHTML self aware and welcome people by its own but right now all I can offer is a blank page with only hyperHTML library in as script.

You can test in that console anything you want though. hyperHTML will be already available.

hope I don't get blocked because of the limitations of selectors

theoretically you could just overwrite Element.prototype.querySelectorAll and put any selector library you want, but I agree this bit should be improved or, at least, documented.

dsadhanala commented 6 years ago

yes, I'm planning to make hyperHTML self aware and welcome people by its own but right now all I can offer is a blank page with only hyperHTML library in as script.

my bad, I didn't know that you mean use console as this page has hyperHTML already loaded :)

theoretically you could just overwrite Element.prototype.querySelectorAll and put any selector library you want

do you have any examples or reference I can use?

WebReflection commented 6 years ago

so, this specific issue has been fixed (attr.ownerElement not needed anymore with class case)

I will open a new issue about exposing a way to change internal selector and use something else

WebReflection commented 6 years ago

https://github.com/WebReflection/basicHTML/issues/13

WebReflection commented 6 years ago

btw, minimum requirement to use Sizzle via node is this one:

const {Document, Element} = require('basichtml');

global.window = global;
global.document = new Document();
const Sizzle = require('sizzle');

Element.prototype.querySelectorAll = function (css) {
  return Sizzle(css, this);
};
dsadhanala commented 6 years ago

I'll give it a try, thanks for the quick turn around!

WebReflection commented 6 years ago

@dsadhanala I have simplified initialisation with the ability to include a custom selector through the .init(...) method as described in the README: https://github.com/WebReflection/basicHTML#new-init-in-013

dsadhanala commented 6 years ago

@WebReflection Thanks for the update!

I will open a new issue with few suggestions.