Closed Bug 48974 Opened 24 years ago Closed 23 years ago

Need mechanism for multi-file JS components

Categories

(Core :: XPConnect, defect, P3)

Product:
Core
Component:
XPConnect
Type:
defect

Tracking

()

Status:
RESOLVED FIXED

People

(Reporter: aaronlev, Assigned: rginda)

References

Details

Attachments

(10 files)

iotest.js; runtime js file inclusion, implemented in js
643 bytes, text/plain
Details
xpc.diff; Components.load, implementd in C++
5.57 KB, patch
Details | Diff | Splinter Review
DIFF for io.js
1.43 KB, patch
Details | Diff | Splinter Review
xpc.diff; second attempt
6.54 KB, patch
Details | Diff | Splinter Review
js/src/xpconnect/loader/mozJSSubScriptLoad.cpp
6.30 KB, text/plain
Details
js/src/xpconnect/loader/mozJSSubScriptLoad.h
1.88 KB, text/plain
Details
mozJSComponentLoader.cpp
34.99 KB, text/plain
Details
loader.zip, the entire mozilla/js/src/xpconnect/loader directory
24.94 KB, application/octet-stream
Details
mozJSSubScriptLoader.cpp
10.07 KB, text/plain
Details
updated mozJSSubScriptLoader.cpp
10.18 KB, text/plain
Details 
We need the ability to include, or import javascript from inside another 
javascript file.
To what context are you referring?  In the JS shell (and xpcshell/lcshell
variants), there is a ``load'' command.  Are you loading a script into content
with <script src=>?  A JS component?

The JS component case is a little trickier, because:

 - you have to detect file changes across all included files when you're
figuring out when to re-register (maybe not a problem, long term)

 - you have to specify the name of the included file somehow, which gets a
little tricky.  If you use an URL (or relative-URL) syntax, you have to have
necko stuff available when you're executing the component script, such as at
registration time.  If you use a relative path name, you're constraining
yourself to local files (perhaps not a big deal now; webapp stuff might make it
annoying later), and you have all sorts of XP-file-path issues to face.

I'm not sure what I think about this.  Doing ``import'' for JS components and
hoping that relative file paths work -- or waiting for rayw's search-path work
to land? -- is a little tempting, but I fear making a hash of things.

Much on this topic was already discussed in bug 14949
Status: UNCONFIRMED → NEW
Ever confirmed: true
Summary: Can't use import statement in Javascript → Need mechanism for multi-file JS components
Yeah, that one covers the in-chrome case pretty thoroughly, but I'm not sure the
issues are the same for JS components, so I'm re-titling this one and keeping it
open.

I'm starting to need this in a bad way for chatzilla.  How about a component 
with an interface something like...

interface nsIScriptLoader {
  bool loadURL (in wstring url);
  bool loadFile (in nsIFile file);
};
Why a new interface (service too, I guess)?  If you can use nsILocalFile, 
nsIDirectoryService, and eval(), go for it.

/be
If it doesn't work with relative paths (url or local file) then it is broken. 
The last thing we want is people being forced to use absolute paths to load 
their library code.
I think I'd prefer a component-context API function, if only so that it could
eventually co-operate with the chrome script cache (or one of its own) to avoid
compiling multiple copies of the same script (IO.js or string-bundle.js, for
example).

The question was put: what context am I using the scripts in?
When designing a large app that will utilize many javascript function 
libraries, it is better to list those dependencies within the source javascript 
file itself, then in each XUL file. I am not familar with the load() function.
Aaron, load() works in xpcshell. 

load('myLib.js');

xpcshell is part of debug builds. 

For now, having a simple load() top level method that automatically deals w/
file url, chrome, local and full path issues would be great.

But if no one has time, then i guess we can do what Brendan suggests.
Perhaps as a top level method w/ a simple API that will handle these details of
the various flavors of local file paths.

So then we can just do a:

include('myLib.js');

This method can also return if any dupe files are passed to it . . .

It probably wouldn't take very long to do.

pete


Here's two whacks at a solution...

iotest.js doesn't associate a filename with the new script, so error messages
would be pretty hard to decipher.  Also, you'd have to cut n' paste this into
the first js file that needed to include another js file.

xpc.diff adds wacky dependancies to xpconnect, check out the new #includes in
xpccomponents.cpp (nsIResChannel.h is bogus, I forgot to remove it from the
diff), but you get better error reports.  Also, we should probably set some
principals on the new script, and throw compile time errors back to the caller,
instead of returning NS_ERROR_FAILURE.
Rob, i couldn't get your first example to work.
I added an include function to io.js w/ error checking, etc . . .

However, eval is not working. Anyone know what i am doing wrong here?

Attaching now . . .

pete
Attached patch DIFF for io.js — — Splinter Review 

    
    

    
  
Here is my test:

js> 
load('/usr/src/THEME_BUILDER/Theme_Builder/theme_builder/theme_builder/content/js/io.js');
js> var file = new File();
js>
file.include('/usr/src/THEME_BUILDER/Theme_Builder/theme_builder/theme_builder/content/js/test.js');
function pete() { dump("\n\n\n\n********** TEST ***************\n\n\n\n"); }
js> pete();
typein:4: ReferenceError: pete is not defined



    
    

    
  
Rob, here is your code error.
I just pasted it in . . . 

----------------------------------------------------

js> const IOS_CID = "{9ac9e770-18bc-11d3-9337-00104ba0fd40}";
js> const nsIIOService = Components.interfaces.nsIIOService;
js> const SCRIPTABLEIS_PROGID = "component://netscape/scriptableinputstream";
js> const nsIScriptableInputStream =
Components.interfaces.nsIScriptableInputStream;
js>
js> function loadURL (url)
{
    var ios = Components.classesByID[IOS_CID].createInstance(nsIIOService);
    var chan = ios.newChannel (url, null);
    var is = chan.openInputStream();
    var sis =
Components.classes[SCRIPTABLEIS_PROGID].createInstance(nsIScriptableInputStream);
    sis.init (is);
    var script = sis.read (chan.contentLength);
    return eval(script);
}
js>
loadURL('/usr/src/THEME_BUILDER/Theme_Builder/theme_builder/theme_builder/content/js/test.js');
uncaught exception: [Exception... "Component returned failure code: 0x804b000a
[nsIIOService.newChannel]"  nsresult: "0x804b000a (<unknown>)"  location: "JS
frame :: typein :: loadURL :: line 9"  data: no]
js>



    
    

    
  
the onbly way i can get this to work is by doing this:


js> load('/usr/src/M18/mozilla/xpcom/tests/utils/io.js');
js>  var file = new File();
js>
eval(file.include('/usr/src/THEME_BUILDER/Theme_Builder/theme_builder/theme_builder/content/js/test.js'));
js> pete();
********** TEST ***************js>
js> 


Anyone know why returning an eval doesn't behave as would be expected?




    
    

    
  
Sorry, I guess I should have specified that these routines load file:,
resource:, and chrome: urls ONLY.  If you prefix your path with "file://" it
should work as expected.  Though, I don't think you'll be happy about it.  Any
errors generated by the included script will be quite hard to track down.

On another note, I spoke with jband about xpc.diff, and since he's too shy to
make his comments here, I'll try to paraphrase...

1) The new dependencies are bad, he'd prefer that we only create the load()
method if the required components exist.

2) jband is morally opposed to checking in anything that can't do relative
paths.  He doesn't consider resource: and chrome: as meeting this criteria.

RE point 1, it sounds like we'll end up creating a script-loader component
anyway, so why not just component-ize this code, and leave it to the script
writer to instantiate the component.

RE point 2, IMO, this is a case for Worse is Better, but I'm biased.  I'd love
to have this for chatzilla.  Relative paths could be added to the component at a
later time w/o affecting current users, who would be using fully qualified
urls.  Also, in the context of mozilla, chrome: and resource: urls are relative
enough for most purposes (like, mine for example.)  Relative http: paths are
moot, as this routine doesn't load from http:.

    
    

    
  
If we're willing to apply Allman's law one more time, I think we can avoid the
dependencies without requiring (smaller, granted) boilerplate JS
copied-and-pasted everywhere.

How about the Load::call method tries to load a subscript-loader component by
progID the first time it's called, and then calls through to it if it's able. 
If it's not able to create the subscript-loader, it just bails with
NS_ERROR_UNAVAILABLE or whatever.

Our subscript-loader component would be part of the component loader, but only
built/installed ifndef XPCONNECT_STANDALONE, which means that it could also do
the privilege management.  Whaddya think?

Hrm.  The privilege management thing is actually a little scary in that model. 
Maybe the loader should just, ifndef XPCONNECT_STANDALONE, annotate the
Components object with the load() thingy.  The loader is already lightly sprayed
with XPCONNECT_STANDALONE conditionals, so what's one more?

Whaddya think?

    
    

    
  
For the here and now . . 

this is what i have for io.js. It will handle local file paths.
It registers included files so you don't use them again etc. . . 

However like i said, i can't get eval to work properly. So i am currently just
retuning the read as a string and using eval from the caller . . .

/********************* INCLUDE **************************/
include : function (path) {
  if(!path) {
  dump("missing argument\n\n");
  return;
  }
  if(this.leaf(path)=="io.js"){
  dump("Sorry, can't include io.js because you are already using it!\n\n");
  return;
  }
  if(!this.exists(path)) {
  dump("invalid path\n\n");
  return
  }
  if(this.includeFiles) {
    for(var i=0;i<this.includeFiles.length;i++){
      if(path==this.includeFiles[i]){
      dump("Already using this include file, ignoring . . .\n\n");
      return;
      }
    }
  }
  this.includeFiles.push(path);
  var retval;
  this.open(path)
  retval    = this.read(path);
  return retval;
},
/********************* INCLUDE **************************/


    
    

    
  
Wouldn't it be better to just use the DOM to do a HTTP GET, and then use an eval
() nested in a try{}?

    
    

    
  
Hmm...i misuderstood, you want to load resource: and chrome: .Ignore previous 
comment.

    
    

    
  
So, if we're going to #ifdef XPCONNECT_STANDALONE, why make a new component when
we can just #ifdef out the body of the function (and the includes) and replace
it with a NS_ERROR_UNAVAILABLE.  Jband seems to think this would be ok, although
he still thinks including necho stuff in xpconnect is unpure (He half-jokingly
sugested it be a component living in the js-component-loader .so)

Anyway, I'll rework the patch to include the #ifdefs, walk the scope chain
instead of using JS_GetGlobalObject(), and report failures as js exceptions
instead of XPCOM failures, but I'll need help with the principals stuff
(shaver?)

    
    

    
  
Why was jband half-joking?  Let's make the JS component loader put its Load
object in place on the Components object when it bootstraps XPConnect.  Localize
the dependencies, etc.

The principal stuff is the other reason; what principal would you give something
that gets Components.load()ed?  The principal from the calling script?  Let's
make the component loader put that policy in place for its scripts, and maybe
save ourselves a firedrill later on.


    
    

    
  
I agree with jband (not joking) and shaver (i think, hope I'm following this all
accurately): we should make a separate component and not wire up necko into a
ball of string with XPConnect.

/be

    
    

    
  
Making the js component loader strap the load() function to the Components
object (e.g. with JS_DefineFunction) wouldn't expose the load() function to
chrome.  This may or may not be a good thing.  I'd probably argue the latter.

Turning the code into an nsIXPCScriptable component living the the js component
loader module WFM.  XPConnect can attempt to instantiate the component (by
progID, as shaver said) in nsXPCComponents::GetLoad(), and throw an error if
that instantiation fails. 

comments?

    
    

    
  


  

    
    

    
  
I used the CLSID instead of the progID, but the rest is my interpretation of
what we've discussed.

    
    

    
  
I don't care about solving the exposure-to-chrome issue right now.

I chose ``progID'' on purpose, because it would allow XPCONNECT_STANDALONE users
to provide their own load() implementation, which seems like a nice thing.

I'll review your patch now...


    
    

    
  
ah.  Good point.  I've changed it to a progID here.
What else?  (the new script still has no principals.)

    
    

    
  
A few things...

- The reason I was on;ly half serious about putting this code in the jsloader 
was because it didn't seem that much better to saddle the loader with this necko 
dependency than to saddle xpconnect. But, everyone seems to be fine with this. 
whatever.

- Like rginda mentioned, he still needs some principals to compile his script. 
This reminds me of somethng that bothers me a lot about this. He supports 
passing in a scope object for the compilation. But, we've given JS loader 
objects system principals. This means (I think) that if one has the specific 
transient privileges to both get access to some JS loader loaded object (e.g. 
window.sidebar that *everyone* can get) and to call Components.load, then one 
can load code that will end up with system principals and which will never be 
checked for privileges at all. This seems broken to me.

- I'm not real clear on why this is Components.anything - i.e. what does a JS 
file loader have to do with Components? Why not require JS code to 
getService for the loader service and then call 'load'. This allows you to get 
rid of the nsIXPCScriptable junk (for which you've added no security check BTW).

- again, I think it sucks that we might leave this as a way to load fully 
qualified urls without any mechanism to do relative paths (which take into 
account the page (and/or! location of the .js file) from which they are called. 
This is in large part a mechanism to allow library code to load other library 
code. Needing to know absolute urls (even if they are remapped chrome: urls) 
sucks. But, I bend to worse is better arguments as necessary.

Anyway, what is so bad about having people write:

function load(url, obj) {
  // XXX get privs here if required
  var iface = Components.interfaces.nsIJSSubScriptLoader;
  var clazz = Components.classes['whatever'];
  var loader = clazz.getService(iface);
  return loader.load(url,obj);
}

The service implementation can use nsIXPCCallContext to deal with optional 
JSObject param and to get at the JSContext. The security check on whether or not 
hte user is allowed to call this particular method of this interface is done 
automtically.

Yes, this is more JS code that users would have to write. Is that so bad?

And then... Why not have window.load ? Why should xpconnect privileges come into 
play? Shouldn't this really be about a codebase check; i.e. this file can load 
that file or not.

I realize that Rob wants to get something working so that he can get on with his 
work. But I really don't want to see us institute something that we'll regret.
Group: netscapeconfidential?

    
    

    
  
Make that 'window.loader' and it can implement 'load' and whatever else we're 
likely to think of in the future. The JS Component loader can have its own 
'loader' object hanging off its global. We can add security checks (somehow?) on 
the parent object that gets passed in and decide whether or not we want to allow 
it to be used as the parent for the code to be compiled. We could even preclude 
passing an explicit parent object when called from content JS.

    
    

    
  
I've reworked this to be a standalone component, included in the jsloader
binary, and using nsIXPCCallContext instead of the scriptable stuff.  I'm going
to use it as part of a project I'm playing with and see how it all "feels", I'll
post new patches when it's a little more shaken out.

Depends on: 50447

    
    

    
  
rginda's gonna do this, it seems.  I'm just going to watch. =)

Assignee: shaver → rginda
Status: ASSIGNED → NEW

    
    

    
  
Accepted bug.  Why was this set to "nscp confidential?"... unsetting.
Group: netscapeconfidential?
Status: NEW → ASSIGNED

    
    

    
  
Rob, It looks like I toggled that without noticing. I've inadvertanly tabbed to 
that box before without catching it right away. I guess I did this here without 
noticing at all.

    
    

    
  
Quick crude way that works
--------------------------
=> Can it be simplified further?

function jsInclude(path)
{
  try {
    var fileInst                = new FilePath(path);
    var fileChannel             = new FileChannel();
    var inputStream             = new InputStream();
    fileChannel.init(fileInst, 0x01, fileInst.permissions);  /* Open to read */
    var inStream = fileChannel.openInputStream();
    inputStream.init(inStream);
    eval(inputStream.read(fileInst.fileSize));
    inStream.close();
    }
  catch (e) {
    dump("jsInclude ERROR: "+e+"\n\n");
    }
}

jsInclude("includethis1.js");



    
    

    
  
Oops - forgot to set these consts at the top.
Here's the whole simple solution again.

/////////////////////////////////////////////////////////////////////////
const FilePath    = new Components.Constructor( "@mozilla.org/file/local;1",
"nsILocalFile", "initWithPath");
const FileChannel = new Components.Constructor(
"@mozilla.org/network/local-file-channel;1", "nsIFileChannel" );
const InputStream = new Components.Constructor(
"@mozilla.org/scriptableinputstream;1", "nsIScriptableInputStream" );

function jsInclude(path)
{
  try {
    var fileInst                = new FilePath(path);
    var fileChannel             = new FileChannel();
    var inputStream             = new InputStream();
    fileChannel.init(fileInst, 0x01, fileInst.permissions);  /* Open for reading */
    var inStream = fileChannel.openInputStream();
    inputStream.init(inStream);
    eval(inputStream.read(fileInst.fileSize));
    inStream.close();
    }
  catch (e) {
    dump("jsInclude ERROR: "+e+"\n\n");
    }
}

jsInclude("/home/aaron/fred.js");


    
    

    
  
Sorry for the bugspam, one more correction:
Make jsInclude return a string of what you want to eval outside of jsInclude.
So you'll end up doing:
eval(jsInclude("include-me.js"));

Otherwise all of your functions and consts you define will be destroyed after
jsInclude returns, because of scope.


    
    

    
  
I'm having 2 problems with the read-file/eval it method just derscribed:
1. Javascript warnings and error messages give the wrong file and line numbers,
so problems are much harder to track down.
2. I have the main component in dist/bin/components, but where should I store
the include files? I don't want them in the same directory, otherwise they get
initialized on startup.

    
    

    
  
Solution to file location.
As suggested by John Bandauer, I named the include files with a .jsl extension
(javascript library file) and stuck them in the components directory along with
the .js component that uses them.
I also had to change the js code that includes them to now find the proper
directory that they're in.
We still have the problem with the line numbers being incorrect in error/warning
messages.

Here's the complete code, that includes files from the components directory:

////////////////////////////////////////////////////////////////////////////////////////////
//  Include javascript files - this works until a better multi-file js mechanism
is achieved
/////////////////////////////////////////////////////////////////////////////////////////////

// First get include directory name
var dirServiceProvider =
Components.classes["@mozilla.org/file/directory_service;1"].
   getService().QueryInterface(Components.interfaces.nsIDirectoryServiceProvider);
var persistent = new Object();
var includeDir = dirServiceProvider.getFile("ComsD",  persistent).path;

// This returns the include file from the include directory as a string
function jsInclude(includeFileName)
{
  var path=includeDir+"/"+includeFileName;
  const FilePath    = new Components.Constructor( "@mozilla.org/file/local;1",
"nsILocalFile", "initWithPath");
  const FileChannel = new Components.Constructor(
"@mozilla.org/network/local-file-channel;1", "nsIFileChannel" );
  const InputStream = new Components.Constructor(
"@mozilla.org/scriptableinputstream;1", "nsIScriptableInputStream" );
  try {
    var fileInst                = new FilePath(path);
    var fileChannel             = new FileChannel();
    var inputStream             = new InputStream();

    fileInst.permissions=0x0755;
    const READ_MODE=0x01;
    fileChannel.init(fileInst, READ_MODE, fileInst.permissions);  /* Open for
reading */
    var inStream = fileChannel.openInputStream();
    inputStream.init(inStream);
    var ret=inputStream.read(fileInst.fileSize);
    inStream.close();
    }
  catch (e) {
    dump("jsInclude ERROR: "+e+"\n\n");
    return "";
    }
  return ret;
}

// Now go get the files and eval them
eval(jsInclude("nsAccessibleState.jsl"));
eval(jsInclude("nsAccessibleCommands.jsl"));

//////////////////////////////////////////////////////////////////////////


    
    

    
  
It's been a while, but better late than never, I guess.

Attached after this comment is a patch that adds a new component,
"@mozilla.org/moz/jssubscript-loader;1".  The component lives in the
js-component-loader library.  It has a single method, loadSubScript(), that
takes as parameters a URL to load, and optionally (through the magic of
nsIXPCNativeCallContext) an object to use as a global during the eval.  The URL
*must* refer to a local file, and use a file:, resource:, or chrome: scheme. 
This component is only intended to be called from JavaScript code.

Loaded scripts are given the system principals, and no caching of compiled
scripts is done.

Because the loader adds dependencies on nsIChannel and nsIIOService, it can be
removed from the build by #define-ing NO_SUBSCRIPT_LOADER or XPCONNECT_STANDALONE.

Attached is the main file, mozJSSubScriptLoader.cpp for online review, and a
.zip of the entire mozilla/js/src/xpconnect/loader subdirectory for more
complete review and testing.

Mac build goop still needs to be added, I'll need some help there.

looking for some hot s?r= action.

    
    

    
  


  

    
    

    
  
um, oops.  I attached the wrong .cpp file, here comes the right one.

    
    

    
  


  

    
    

    
  
I dig it with a vengeance, and want it for 0.8.1.  sr=shaver, let's get some r=
and slip this baby in.


    
    

    
  
attaching updated mozJSSubScriptLoader.cpp, fixes include:

NS_WITH_SERVICE->do_GetService changes
text for null |buf|

In addition, I'll be chkecing the idl file into xpconnect/idl, instead of
xpconnect/loader/idl.

The 0.8.1 deadline is today!  Still looking for a r=, jband, brendan, bueller?

adding peterv to cc list for mac build mods.


    
    

    
  


  

    
    

    
  
fix checked in.
Status: ASSIGNED → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED

    
  
Component: XP Miscellany → XPConnect

          You need to log in
          before you can comment on or make changes to this bug.
        






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: