704720 - Allow multiple 'lib', 'test' etc directories

Status: RESOLVED INVALID

(Add-on SDK Graveyard :: General, defect)

Description

[Mark Hammond [:markh] [:mhammond]]

The openwebapps project would find it very useful to allow multiple directories as the 'lib' directory, so a construct like "require('foo')" would search for the foo module in more than one place.  The use-case is to allow a single .js module to be used both in a jetpack and in a non-jetpack environment (eg, in a server app) without resorting to symlinks (bad on Windows) or duplicate files (bad everywhere).

I'm attaching a patch that allows a package.json to specify, eg:

    "directories": {
        "lib": ["lib", "lib2"]
    }

And have both the 'lib' and 'lib2' directories - both relative to the dir with package.json - searched for modules in the package.

Comment 1

[Mark Hammond [:markh] [:mhammond]]

Attachment: Pointer to Github pull request: https://github.com/mozilla/addon-sdk/pull/272

Pointer to Github pull-request

Comment 2

[Mark Hammond [:markh] [:mhammond]]

Comment on attachment 576385 [details]
Pointer to Github pull request: https://github.com/mozilla/addon-sdk/pull/272

In IRC, Myk said he is probably OK in principle and would like feedback from Gozala and/or Warner - so both you guys get to share the love :)

Comment 3

[Myk Melez [:myk] [@mykmelez]]

@warner, @gozala: this implements a "library search path" for modules similar to the one by which shared libraries are located on Unix systems, which seems useful and reasonable.

It's also a step in the direction of getting rid of packages, since it makes it possible to access modules in multiple directories without the hassle of having to turn those directories into valid packages and then reference them via --extra-packages or --package-path.

We could implement this via a command line flag (like --module-path) instead of a package.json option.  And a completely different approach would be to allow require() statements to reference modules that aren't in a package, in which case the OWA addon would simply require("../../../sync/sync.js").

I don't have strong feelings about which approach is best, and y'all know the module resolution code better than I do, so I'd like to get feedback from at least one of you on this implementation.

Comment 4

[Irakli Gozalishvili [:irakli] [:gozala] [@gozala]]

(In reply to Mark Hammond (:markh) from comment #0)
> The openwebapps project would find it very useful to allow multiple
> directories as the 'lib' directory, so a construct like "require('foo')"
> would search for the foo module in more than one place. 

I'm bit concerned about adding such a feature for a couple of reasons:

1. It makes it less explicit what `require('foo')` actually means without looking into some 'PATH' variable, which means that code might work perfectly well in one environment and break completely in other if `PATH` variable is not exactly same.
2. Node.js decided to gave up on such features for in favor of simplicity that turned out pretty well IMO.
3. I'm afraid that it's not very compliant with the way js native js modules are developed, as there will be no search there, mainly because it's not very complient with web.  

> The use-case is to
> allow a single .js module to be used both in a jetpack and in a non-jetpack
> environment (eg, in a server app) without resorting to symlinks (bad on
> Windows) or duplicate files (bad everywhere).
> 

Could you elaborate a bit how does multiple libs help here ? I'm assuming that lib order will be different on platform, if so you'll end up with diff package.json no ?

I think it's better to employ requirejs's approach to do this which James described in this thread (Cc-ing him here):

https://groups.google.com/forum/#!topic/mozilla-labs-jetpack/AJTts8VBPxw 

> I'm attaching a patch that allows a package.json to specify, eg:
> 
>     "directories": {
>         "lib": ["lib", "lib2"]
>     }
> 
> And have both the 'lib' and 'lib2' directories - both relative to the dir
> with package.json - searched for modules in the package.

Comment 5

[Mark Hammond [:markh] [:mhammond]]

(In reply to Irakli Gozilalishvili [:irakli] [:gozala] from comment #4)

> 1. It makes it less explicit what `require('foo')` actually means without
> looking into some 'PATH' variable, which means that code might work
> perfectly well in one environment and break completely in other if `PATH`
> variable is not exactly same.

To be clear, it doesn't use a PATH variable or anything from the environment - the search paths are specified in package.json:

    "directories": {
        "lib": ["lib", "lib2"]
    }

So should work the same across all environments.

> 2. Node.js decided to gave up on such features for in favor of simplicity
> that turned out pretty well IMO.
> 3. I'm afraid that it's not very compliant with the way js native js modules
> are developed, as there will be no search there, mainly because it's not
> very complient with web.

Note that the searching is all done at xpi build time, not runtime - so in effect it just offers flexibility in how modules are laid out on the file-system.  Once inside the XPI, all modules are put in the same place.

> Could you elaborate a bit how does multiple libs help here ? I'm assuming
> that lib order will be different on platform, if so you'll end up with diff
> package.json no ?

This isn't about cross-package modules, just how the file-system can be laid out.  The problem I'm trying to solve is we currently have a file-system like:

./server # eg, a node.js server
./server/lib/repo.js # an implementation file for the server.
./jetpack # the jetpack.
./jetpack/lib/repo.js # Identical to repo.js above

The 2 copies of the same repo.js is what I'm trying to fix without duplicating the file or using symlinks.  With my patch, the idea is that jetpack/lib/repo.js can be removed, and jetpack/package.json can specify that the "lib" files can be found in both "./lib" and "../server/lib".

> I think it's better to employ requirejs's approach to do this which James
> described in this thread (Cc-ing him here):
> 
> https://groups.google.com/forum/#!topic/mozilla-labs-jetpack/AJTts8VBPxw 

I wonder if there isn't a mis-communication here - my proposal is just used at buildtime to allow modules in a single jetpack to come from 2 directories on the file-system.

Comment 6

[Irakli Gozalishvili [:irakli] [:gozala] [@gozala]]

(In reply to Mark Hammond (:markh) from comment #5)
> (In reply to Irakli Gozilalishvili [:irakli] [:gozala] from comment #4)
> 
> > 1. It makes it less explicit what `require('foo')` actually means without
> > looking into some 'PATH' variable, which means that code might work
> > perfectly well in one environment and break completely in other if `PATH`
> > variable is not exactly same.
> 
> To be clear, it doesn't use a PATH variable or anything from the environment
> - the search paths are specified in package.json:
> 
>     "directories": {
>         "lib": ["lib", "lib2"]
>     }
> 
> So should work the same across all environments.
> 
> > 2. Node.js decided to gave up on such features for in favor of simplicity
> > that turned out pretty well IMO.
> > 3. I'm afraid that it's not very compliant with the way js native js modules
> > are developed, as there will be no search there, mainly because it's not
> > very complient with web.
> 
> Note that the searching is all done at xpi build time, not runtime - so in
> effect it just offers flexibility in how modules are laid out on the
> file-system.  Once inside the XPI, all modules are put in the same place.
> 
> > Could you elaborate a bit how does multiple libs help here ? I'm assuming
> > that lib order will be different on platform, if so you'll end up with diff
> > package.json no ?
> 
> This isn't about cross-package modules, just how the file-system can be laid
> out.  The problem I'm trying to solve is we currently have a file-system
> like:
> 
> ./server # eg, a node.js server
> ./server/lib/repo.js # an implementation file for the server.
> ./jetpack # the jetpack.
> ./jetpack/lib/repo.js # Identical to repo.js above
> 
> The 2 copies of the same repo.js is what I'm trying to fix without
> duplicating the file or using symlinks.  With my patch, the idea is that
> jetpack/lib/repo.js can be removed, and jetpack/package.json can specify
> that the "lib" files can be found in both "./lib" and "../server/lib".
> 
> > I think it's better to employ requirejs's approach to do this which James
> > described in this thread (Cc-ing him here):
> > 
> > https://groups.google.com/forum/#!topic/mozilla-labs-jetpack/AJTts8VBPxw 
> 
> I wonder if there isn't a mis-communication here - my proposal is just used
> at buildtime to allow modules in a single jetpack to come from 2 directories
> on the file-system.

I thought you wanted to use package.lib in a similar way to PATH env variable.

Still, I don't quite understand why do you cave to duplicate repo.js ? Can't you just specify dependency on 'server' package ?

Comment 7

[Mark Hammond [:markh] [:mhammond]]

After chatting with gozala on IRC, we agreed to look for other alternatives - we could always come back to this if they don't work out.

Comment 8

[Brian Warner [:warner :bwarner]]

Comment on attachment 576385 [details]
Pointer to Github pull request: https://github.com/mozilla/addon-sdk/pull/272

clearing r? request since this was INVALIDed