Closed Bug 48974 Opened 24 years ago Closed 24 years ago

Need mechanism for multi-file JS components

Categories

(Core :: XPConnect, defect, P3)

defect

Tracking

()

RESOLVED FIXED

People

(Reporter: aaronlev, Assigned: rginda)

References

Details

Attachments

(10 files)

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 (deleted) — 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?
Attached patch xpc.diff; second attempt (deleted) — Splinter Review
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.
Attached file mozJSComponentLoader.cpp (deleted) —
um, oops. I attached the wrong .cpp file, here comes the right one.
Attached file mozJSSubScriptLoader.cpp (deleted) —
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.
Attached file updated mozJSSubScriptLoader.cpp (deleted) —
fix checked in.
Status: ASSIGNED → RESOLVED
Closed: 24 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: