Skip to content
This repository has been archived by the owner on Jun 15, 2021. It is now read-only.

Multiple includes with relative require()s #120

Closed
rvagg opened this issue Feb 9, 2012 · 16 comments
Closed

Multiple includes with relative require()s #120

rvagg opened this issue Feb 9, 2012 · 16 comments

Comments

@rvagg
Copy link
Member

rvagg commented Feb 9, 2012

Just recording this while I think of it.

Since people seem to assume that Ender supports require('./relative') and can bundle multiple files into a single build, why not support it?

Some package.json adjustments would be required. maybe:

  "main": [ "dist/module1.js", "dist/module2.js", "dist/module2.js" ]

or

  "ender": {
    include: [ "dist/module1.js", "dist/module2.js", "dist/module2.js" ],
    bridge: "dist/ender.js"
   }

If you have multiple includes then it wraps the whole lot with a version of require() that can resolve local names but those names aren't available outside of the bundle. e.g. dist/module1.js could call require('./module2.js') but you couldn't do the same from another ender package. It could even be made aware of the relative locations of the files so you can require('../lib/file.js') from one location and require('./file.js') from the same directory as file.js and resolve to the same module depending on what module it was called from -- making it pretty much the same as what people expect from Node.

@ghost ghost assigned rvagg Feb 9, 2012
@nleush
Copy link

nleush commented Feb 28, 2012

It could be great feature!

@rvagg
Copy link
Member Author

rvagg commented Feb 28, 2012

FYI this is on the top of my list when I get to doing new stuff, the more I think about it the more I want to have this for my own work, completely replace existing build steps and let you develop browser code just like you would for Node.

@vic
Copy link

vic commented Mar 9, 2012

See ender-js/ender-js#13 and #126

@enyo
Copy link

enyo commented May 22, 2012

That would be so awesome. Actually.. what is the current way of including multiple files?

@rvagg
Copy link
Member Author

rvagg commented May 22, 2012

If you're asking about a standard module, then in in 0.8.x the "main" in package.json can take an array of files that is simply concatenated together. In 1.0-wip both "main" and "ender" can take arrays of files that are concatenated together.
If you're doing a bundle of your own scripts with an Ender build then you can use the ender compile command which will concatenate ender.js with any files you specify and then run it through Closure compiler. 1.0-wip now takes a --level argument so you can compile with SIMPLE_OPTIMIZATIONS if you want (--level simple from memory).

@enyo
Copy link

enyo commented May 29, 2012

I had assumed that ender includes the files referenced in main in the same order as they are defined. But apparently the order is random. Is there a way to force the order of those files?
I ask because the second file in my main list depends on the first one.

@rvagg
Copy link
Member Author

rvagg commented May 29, 2012

Should be fixed in the 1.0-wip branch @enyo. Install with npm install -g ender@dev or check out from the repo and npm link.

@enyo
Copy link

enyo commented May 29, 2012

Thanks a lot! That did it...
I'm sorry to ask questions like this in here, but I haven't found an IRC or mailing list for ender yet...

@rvagg
Copy link
Member Author

rvagg commented May 29, 2012

That's fine. We're using this as a forum for support at the moment, nothing else has stuck so feel free to open issues or comment in others if you have questions. Or bug someone on twitter.

@niclashoyer
Copy link

Maybe we could make the adjustments of package.json fields optional, if we're using detective to find all local requires?

@rvagg
Copy link
Member Author

rvagg commented Jun 12, 2012

@niclashoyer I've thought a bit about this and my current opinion is that I'd like to avoid magic as much as possible. I'd rather have package authors be explicit about which files are included.

Perhaps I could be persuaded by the weight of others' opinions though so will try to keep an open mind going forward.

@amccollum
Copy link
Contributor

I have implemented a rough version of this feature.

The current implementation is probably fairly brittle, but it required very little modification to the existing codebase. It needs more work (and testing!) before it can really be considered merge-able. I ended up calling the key internal instead of include in my implementation, but I don't feel strongly about either name.

There are issues if you also allow arrays in the main key, because it no longer knows what path to use as a base for relative requires. It would probably have to detect this and throw an error if you tried to mix and match internal with an array of main scripts in different directories.

Of course, this wouldn't be an issue if you just treated the main key as a set of internal requires when it was an array, but you would sacrifice the simplicity of the case where you really do want to just concatenate the scripts together. The overhead of internal requires is not huge (well, if the relative path handling was moved to the main require function, that is), but it's definitely not negligible, either. I suppose I would be slightly in favor of this approach just because I feel like cases where you want pure concatenation are somewhat uncommon.

Any comments either here or inline on the commit would be welcome.

@rvagg
Copy link
Member Author

rvagg commented Sep 30, 2012

Nice, I've had a quick look and it looks like a great start, I'd be keen to have a play with it.

I did imagine that we could cut down the overhead of the duplicated internal look-up stuff by implementing it in the client lib ('ender-js', make global require() and provide() work both global and local where they need to. If you think that might be a way to go then you could experiment by forking ender-js and then using the --client-lib build argument to point to an alternative (can be a path to your local clone of course).

@amccollum
Copy link
Contributor

I went ahead and moved the require mechanics to the client lib.

That allows the passed-in require function in the package templates to be pretty minimal:

    function (path) { return require(path, "{{mainPath}}", "{{packageName}}:"); }

Right after I wrote my last comment, realized why I didn't just go with the solution of converting the main array to the internal requires. The problem is that module.exports becomes somewhat ambiguous. I guess you could do the equivalent of what happens now and just merge all the various exports objects together, but that seems like a fairly large departure from CommonJS semantics, especially if you were actually using submodules for api encapsulation, etc.

I think the next step would be writing some tests, but I'm still wrapping my head around the testing code. In case it helps anyone, here's a gist for a basic package I created for testing.

@alexgorbatchev
Copy link

I wish it would've been stated somewhere on the site that Ender doesn't follow require() statements... It really didn't feel good to have to find this out after an hour of trying to figure out why the rest of my files don't make into the final build file. Is it correct to assume that Ender expects to work with just one JS file per module? That doesn't seem like a realistic scenario to me.

@amccollum
Copy link
Contributor

This is now supported in [email protected].

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

7 participants