It would be good to have a simple function to load additional scripts, maybe like the importScripts() function from Web Workers. You can of course make your own <scripts> now but that's a lot of work which a function could make quite easy.

With this function a sort of "standard library" including things like issue #29 could be included. And of course it would be easier to include regular JS libraries as well.

I'm researching the best way to handle "modules" and "packages".

I think a simple $ = require('jquery', 'http://code.jquery.com/jquery-1.5.min.js') would be nice. The phantom could fetch the url and cache it so that in subsequent callbacks one can simply do $ = require('jquery') and the cached version is return.

Nice to have could be to have a configuration file that maps logical names to urls so they're easier to maintain/upgrade and don't have to be hard-coded (some sort of dependency injection).

Also nice to have would be to have export('myLib', function(){ ...}); so that callbacks can fetch back what they need and we don't need to re-inject the whole script every time.

Note that a lot of things are not really possible right now until http://bugreports.qt.nokia.com/browse/QTWEBKIT-2 is solved.

+1

This is really needed, as including through "document.body.appendChild()" requires your code to "wait" for the library to be parsed by the Javascript Core first.

For devs that are hitting my same barrier and want this feature, here is a temporary fix:

function includeJs(filename, onloadHandler) { updateState("includeJs: " + filename);

var el = document.createElement('script');

el.type = 'text/javascript';
el.onload = onloadHandler || null;
el.src = filename;
document.body.appendChild(el);

}

I guess implementing the Require pattern would be the most optimal solution, since developers are already familiar with that, e.g.

var foobar = require('foobar.js'); foobar.doSomething();

Issue 89 has been merged into this issue.

It would be great if someone can express a preference of which library we should use to implement this pattern. I found this interesting: http://requirejs.org/docs/jquery.html. But there are TONS of Javascript Dependency Management Scripts out there. Some are really big, some are really smart.

What do you think should we pick?

OR... we could just design something simple but effective ourself and implement that. Do you have a page/reference on how exactly the "require()" pattern you mention works? I could find multiple example online, and they sometime variate too much.

Yes, I'm volunteering in doing this :)

I prefer something simple, i.e. it could be just the logical extension of your script loader.

Starting from an example, something like this would be very reasonable:

var crypto = require('crypto'); var fingerprint = crypto.md5sum(some_data);

Also, the trick is the "exports" pattern inside the module itself, i.e. a module will not simply pollute the global object.

Until this is resolved - what would be the best way to load a library eg. jQuery into a loaded page?

Scroll up and see comment #5 for the workaround.

In addition to 5 you can also do this:

void Phantom::loadLibrary(const QString &fileName) { QFile file; file.setFileName(fileName); if (!file.open(QFile::ReadOnly)) { std::cerr << "Can't open " << qPrintable(fileName) << std::endl << std:: endl; exit(1); return; } m_library = file.readAll(); file.close(); m_page.mainFrame()->evaluateJavaScript(m_library); }

You forgot to add that as a public slot. :)

As shown here at my fork: https://github.com/detro/phantomjs/blob/major_refactoring/src/phantom.cpp#L394

@Ariya: so, the 'required' pattern is defined in CommonJS as you probably know (http://www.commonjs.org/). And, as you also said, it requires specific 'exports' in the module itself. All good: clearly CommonJS is the future in terms of "standard library" for Javascript.

But, that is a very specific requirement: jQuery or other libraries are not going to change because of CommonJS (for now I guess). Hence, we still lack a built in way to load JS libraries.

If we follow the supervisor-worker pattern, what you want there is to "inject" any JS library, correct?

Maybe the example would look like:

var page = WebPage.open(url); page.injectScript('/path/to/extra/lib.js');

Hey detronizator, what does updateState do in your snippet from comment #5? Just console.log a message or something similar?

Yes, that snippet comes out of my tests, and was updting the state while writing some console.log.

Ivan, with the new WebPage object now you have the chance to implement the above injectScript solution, perhaps extending with an optional callback (since it is asynchronous).

Patch is welcomed :)

Comment deleted

We were discussing a way to do sequential actions (instead of the verbose async), and found that there are some nice libraries that allow to do this without blocking async. (steps, promise.js, async.js, futures, ..). This would also allow you to split your JavaScript's up into separate modules (could be really useful). So for these reasons I am suggesting we also allow injecting a local JavaScipt into the Phantom object.

And of course, would also love to soon see a way to inject a local / URL based script into a page.

I'd implement it myself were it not for my lack of C++ skills, although I could probably hack something together, I'd rather let someone who knows more do a better job! :)

http://groups.google.com/group/phantomjs/browse_thread/thread/eebcd758f18ad8c4/7f22b94e128b4dee

Just submitted a pull request for "page.loadJsFile()" that works with local paths.

Based on the last previous comment and Ariya answer to my pull request, I'd: - rename to "inject" - build a support for "URL" (even though this will either be "blocking" or "async")

What do you guys think?

I like the name "injectJS", and for the URL JS loader "includeJS".

Did you read my comment #23? There are a few good reasons to also expose "injectJS" to the Phantom object (easy to do, just do a m_page.injectJS(filename)).

Lastly, regarding URL paths, and I see this as a problem; if you use local paths without appending the original script's directory to the path, it becomes like:

./phantomjs ~/Desktop/script.js

script.js: var page = new WebPage(); page.injectJS('library.js');

The problem with this, is that when it searches for library.js, it'll look inside the phantomjs binary folder (or whatever the cwd is). So, if 'library.js' is in the same folder as script.js, you'll have to do:

page.injectJS('/home/user/Desktop/library.js');

This is why I am proposing to prepend the original scripts directory path to the filename before injecting it. The prepend can be omitted if they already have a directory path in the filename (but first make sure the directory path in the filename ISN'T a subdirectory of the folder script.js is in (e.g. a filename like 'libs/script.js'), otherwise we SHOULD prepend it still)

Does that make sense?

1) so, if I get it right, you'd like "injectJS" to be synchronous and only local - "includeJS" to be async (using the tag?)

2) About the paths, I think you are right. The injectJs should prepend the "starting script path" if what's passed is not already a full path. This makes perfect sense for me. I did actually think about it while coding today, but because I was not sure of the best approach, I though I'll show a first implementation, and than tune it based on feedbacks

Ariya?

1)

you'd like "injectJS" to be synchronous and only local I thought it was already synchronous and local?

"includeJS" to be async (using the tag?) I was thinking along the terms of your original includeJS, like.. https://github.com/ariya/phantomjs/pull/11/files#L0R54

-- I really had already assumed this would function similarly to your pull request 11 (link above).

2) Awesome. :)

OH, I almost forgot! When I had the loadScript function on my port, I made it so that people could also load CoffeeScript's as well as JS. It was a nice touch, considering we can already use CoffeeScript's for the main script. Not terribly needed, but would be cool. :)

I was just re-describing the 2 methods. "loadJsFile" (what is going to be "injectJs") is already synchronous.

About the coffee thing, I will be able to do it only for "injectJs" I think.

-- Ivan De Marino Front-End Developer @ Betfair

email: ivan.de.marino@gmail.com | detronizator@gmail.com | ivan.demarino@betfair.com web: blog.ivandemarino.me | www.linkedin.com/in/ivandemarino | twitter.com/detronizator mobile: +44 (0)7515 955 861

I was just re-describing the 2 methods. "loadJsFile" (what is going to be "injectJs") is already synchronous. Ah, I see.

About the coffee thing, I will be able to do it only for "injectJs" I think. Right, there's little/no use for doing it on includeJS anyways. As far as the callbacks go, if you specify a callback in CoffeeScript (in the main file), it'll already have been converted by the time it hits includeJS, so that's a plus. :)

Alright, sounds good. Can't wait to port it over. :)

I remembered something. I think we should also cache the script. This wouldn't do much for regular js files (we wouldn't have to re-open it again though), but for CoffeeScript's it would be dramatic, since it wouldn't need to be reconverted every single time.

Caching with a key:value based system works, where the key is the original filename passed to the script (not the one we altered with adding the start script). I already have my implementation done. I'll post the diff below.

Caching the converted coffee script? Do you think is really worth it?

I believe so. My reasoning is:

On each page request, the javascript would have to be re-executed in the page in order to work. This is fine for regular javascript, but with CoffeeScript, it would really slow down page loading. Since we already converted it once, we shouldn't need to redo it every time, since we already did it.

Rest assured I see why you want to do it. It's just that it's an "how many times would we need it" doubt.

Would anyone write a script that loads the same coffee script a lot of times?

Well, if so, it's a very badly done implementation if you ask me...

-- Ivan De Marino Front-End Developer @ Betfair

email: ivan.de.marino@gmail.com | detronizator@gmail.com | ivan.demarino@betfair.com web: blog.ivandemarino.me | www.linkedin.com/in/ivandemarino | twitter.com/detronizator mobile: +44 (0)7515 955 861

It's just that it's an "how many times would we need it" doubt. Would anyone write a script that loads the same coffee script a lot of times? True, I just didn't see any negligible effects, since it seems fairly easy to implement and the function exits much more quickly (instead of having to re-open the file all the time).

The Phantom object has the effect of caching the script (in it's own way) in m_script.

I guess it's a, "do the benefits outweigh the costs" scenario.

Do you think there are any negligible effects to implementing something like that?

Sorry, not trying to overload you here, just trying to make sure the implementation covers all bases (like in the case of the local file path loading). :)

I'd go to the extent of saying that the caching should be in the "CSConverter" class. I'll see where does that fits.

I'm working on the "includeJs" as well now, and it is slightly trickier than before, given that the whole thing is now split in 2 different contexts, while before there was only one. I'm trying a couple of approaches before submitting my solution.

In the meantime though, take a look to this commit (https://github.com/detro/phantomjs/commit/ce0577adff0f7a28f7b9ef386ec7ae5ea3f5e63a) to see if you think our implementation is equivalent for "injectJs".

Of course, bear in mind that the Coffee Script support is not there yet. ;)

scriptLookupDir() looks nice. Changed mine accordingly.

They seem to be functionally equivalent, minus all my added stuff of course. (CoffeeScript, caching, injectJs in Phantom object) :)

After doing some speed tests, I've determined that caching is (almost) negligible. I found the cause of the speed slowdown, and why caching it was speeding it up.

The reason was that the Converter was the one that needed to be cached (which is really easy), rather than the script. :)

Here's my new diff.

P.S. Fixed a bug with the CoffeeScript converter. https://github.com/Roejames12/phantomjs/commit/3c665681f432601a40e49dc1ae58ea5fe1399784

Since it is encouraged to have small patches, I would argue just create a pull request for what's ready, e.g. injectJS, first.

I will tomorrow. Today I implemented inject and include, but tonight I couldn't submit the pull.

If curious, check out my "utilities" branch: there is a commit with questions that you might be able to answer ;)

Ivan De Marino Front End Developer @ Betfair

Sent from my iPhone 4

Here is an idea is someone wants to pursue it (and beats me to it): a native implementation of includeJS. We can use the network access manager to fetch the script.

Benefit: no need to "pollute" the page DOM (with the script tag)

Well, that for me was the next update for the injectJs. ;)

Ivan De Marino Front End Developer @ Betfair

Sent from my iPhone 4

Looks great! However one thing. The bug I had fixed in this commit is now present again. This only needs to be done to js files, because in CoffeeScript # is a comment, so it'll be ignored. However prepending // for CoffeeScript does not cause it to be a comment, but rather a RegExp (which is unintended!). I'll fix it on my next pull request.

https://github.com/Roejames12/phantomjs/commit/3c665681f432601a40e49dc1ae58ea5fe1399784

What do you guys think of the name "libraryPath."

Rationale: seems that we will just include scripts here, I can't foresee PhantomJS including/injecting anything else. Therefore the "script" prefix is not necessary.

Personally, I also prefer "path" since it seems to be more general.

In addition, if we would support multiple paths (for different lookups), maybe it should be "libraryPaths" with a possible value of array of string (instead of only a single string).

Supporting multiple paths could be a very good idea (operating similar to the PATH environment variable, able to have multiple paths). Definitely useful for having multiple folders of libs elsewhere on the system.

I'm fine with a rename, libraryPaths seems good. :)

What does everyone think about a global "libraryPath", instead of 1 per page object/and the phantom object?

Perhaps we could keep 1 per page object/and the phantom object, and still have a global one?

Is it a good idea?

I'd say, for sake of simplicity, "libraryPaths" for page objects are ALREADY generated out of the same one (i.e. the phantom object's libraryPaths).

Having a global one isn't really going to make a lot of difference. Plus, I confess I like the isolation.

I have in the pipe (once I get some proper home-time to do a nice after-work coding session) a small little library based on Qt. It's going to simplify how to handle command line arguments/options. Something like "getopts", but in a QSettings style. My aim is to than integrate this thing in Phantom, and so make very easy to pass some "configuration parameters" to the phantom object directly from the CLI.

I will change scriptLookupDir to libraryPath soon.

Renaming is fine. But I assume for now ( 1.2) it will work as it already does (I.e. One path). Right?

One path only for 1.2.

Is there anything left to be done for this issue? Otherwise, we shall close it.

I have 2 tasks on my TODO list: - Implement an asynchronous "injectJs" - Add support for multiple entries in the "PATH"

But that can be part of 1.3, as the current stuff in 1.2 is good enough for me.

What about native version of includeJS?

Well, the "callback" part can be done only on the JS side. The appending of the DOM Element "could" be done in C++ for sure but... do we really have to? The amount of JS that is executed there is minimal, and I can't see a real benefit in doing it native, manipulating QWebElements. Maybe I'd save few bits of memory and it would be faster because there would be no JS->C++ "translation" but... really? :)

Unless there is something I completely miss.

If it is possible to implement something without touching the page document, that's the preferred way to do that. I already mentioned the use of network access manager to fetch the script. I'm sure the callback problem can be solved as well.

I tried using the Network Access Manager in my first attempt actually, and the problems was that there is a (for obvious reasons) a delay between "last bit of the resource received" and "resource loaded".

As fast as the JS engine can be, it's still slow enough to break this approach.

That's why I ended up "temporary using" the page signal->slot. And, after all, it's not like anything else is affected, isn't it?

Not sure what you mean by the delay there. Care to elaborate?

Also, your approach is touching the document/DOM. If there is another way to implement the same thing without changing the page itself, that'll be much preferable.

Meanwhile, I'll close this one and we continue on native implementation as a separate issue.

Are we talking about "includeJs" right?

IncludeJS purposely adds a tag to the DOM. That was the purpose. To include stuff without touching the dom, there is the synchronous "injectJs".

Or are we talking about something else here?

Anyway, where do we pick this conversation up?

If the intention of includeJS really to modify the DOM by adding the script tag, then it should be fine.

I guess one day we need to split all these convenience functions to its module, instead of stashing them in bootstrap.js.

yes, but the problem is that it needed a mix of JS and Native code. If it was pure JS...

-- Ivan De Marino Front-End Developer @ Betfair

email: ivan.de.marino@gmail.com | detronizator@gmail.com | ivan.demarino@betfair.com web: blog.ivandemarino.me | www.linkedin.com/in/ivandemarino | twitter.com/detronizator mobile: +44 (0)7515 955 861

This issue has been moved to GitHub: https://github.com/ariya/phantomjs/issues/10032