Skip to content
This repository has been archived by the owner on Oct 3, 2018. It is now read-only.

Support for calling run function in node.js #99

Merged
merged 18 commits into from
Nov 3, 2014

Conversation

4ver
Copy link
Contributor

@4ver 4ver commented Oct 31, 2014

No description provided.

@hassankhan
Copy link
Contributor

Not quite sure if we should have to make changes because jsDom doesn't support what we need. There is an open issue on their repo, maybe we should pester them to get it working.

The only reason I'm hesitant to get rid of TreeWalker is performance-related, last I checked it beats other methods of traversal, but I did check a long time ago 😄

@4ver
Copy link
Contributor Author

4ver commented Nov 2, 2014

The fact that the issue has been open for three years isn't all that encouraging. I don't think the change would affect the performance majorly (could even be faster).

@4ver
Copy link
Contributor Author

4ver commented Nov 2, 2014

I could make it so my tree traversal function is used instead of TreeWalker when it's not available. It would only be an extra 20 or so lines.

@hassankhan
Copy link
Contributor

Yep, I'd be happy with that, the other thing, I'd rather it didn't have to pass a window object in run(), am I missing something?

@4ver
Copy link
Contributor Author

4ver commented Nov 2, 2014

Great! The window object is needed to call functions like document.getElementById, document.createElement and createTreeWalker if it was available. You only need to pass it when it's not available (in node).

@4ver
Copy link
Contributor Author

4ver commented Nov 2, 2014

This is ready to be merged now.

@adam-lynch
Copy link
Contributor

Should the node-only tests be gone now? Maybe we could just have all the tests in the tests directory, with browser.html and rename browser.js to node.js?

@adam-lynch
Copy link
Contributor

Also, you probably need to update CONTRIBUTING.md

@4ver
Copy link
Contributor Author

4ver commented Nov 2, 2014

Moved the tests around there. Not sure where to put the actual tests. Wouldn't be all that nice having a tests/tests directory 😛. Can leave the html file and runner.js in the browser directory though.

@adam-lynch
Copy link
Contributor

Test structure looks good to me

hassankhan added a commit that referenced this pull request Nov 3, 2014
Support for calling run function in node.js
@hassankhan hassankhan merged commit 7d0281e into joypixels:develop Nov 3, 2014
@hassankhan
Copy link
Contributor

Thanks @4ver!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants