search

plumatic / dommy

A tiny ClojureScript DOM manipulation and event library
759 stars 74 forks source link

Compatibility problem in generated code with phantomjs #27

Closed hsalokor closed 11 years ago

hsalokor commented 11 years ago

I'm trying to use PhantomJS in order to run unit tests headlessly, and noticed following issue when upgrading to 0.1.0 from 0.0.2.

PhantomJS apparently does not have Window object in global Javascript scope. This seems to cause the Javascript code with (deftemplate ...) definitions to throw following error on load:

ReferenceError: Can't find variable: Window

By looking at the generated code, the culprit seems to be following lines:

Window.prototype.dommy$template$PElement$ = true;
Window.prototype.dommy$template$PElement$_elem$arity$1 = (function (this$) ...

I tracked it back to PElement definition, and those lines seem to be generated by the js/Window entry in the definition.

Is the definition necessary? I've attached code that removes it, and at least I could not see any major issues with it. Am I missing something?

laughinghan commented 11 years ago

This is necessary in order to do, for example, (dommy/listen! js/window :resize #(resize! my-container))

aria42 commented 11 years ago

We definitely need to listen to js/window changes. What we need is a shim for how to make this work in PhatomJS world. Any ideas what the type is for Window or if there's a way to conditionally not eval that code?

hsalokor commented 11 years ago

Ah, that makes sense. Thanks :)

I'll investigate the issue further w/ phantomjs and get back to this later.

hsalokor commented 11 years ago

I investigated the phantomjs a bit, but I was unable to get anything sensible (like type) from the window or Window implementation.

As a second approach, I just updated the patch to extend the protocol PElement conditionally, i.e. only if js/Window exists. The (try ... (catch ...)) structure was cleanest form I could come up with. It seems to work as expected at least in my limited testing.

It also now prints message to console log if Window is not defined, which should make debugging a bit easier.

cpetzold commented 11 years ago

This seems like a perfectly reasonable solution for now, thanks for the commit! We'll keep headless webkit implementations in mind, because we certainly want to be able to use stuff like phantom as a test platform.