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

Proposal: Ender package.json extensions #131

Open
rvagg opened this issue Apr 17, 2012 · 36 comments
Open

Proposal: Ender package.json extensions #131

rvagg opened this issue Apr 17, 2012 · 36 comments

Comments

@rvagg
Copy link
Member

rvagg commented Apr 17, 2012

I'd like some feedback on extending package.json for additional flexibility.

Current package.json uses

{
    "name": "foo"
  , "main": "foo.js"
}
{
    "name": "foo"
  , "main": "foo.js"
  , "ender": "noop"
}
{
    "name": "foo"
  , "main": "foo.js"
  , "ender": "bar.js"
  , "dependencies": [
        "qwery": "*"
      , "bean": "*"
    ]
}

Also possible:

{
    "name": "foo"
  , "main": [ "foo.js", "baz.js" ]
  , "ender": "bar.js"
  , "dependencies": [
        "qwery": "*"
      , "bean": "*"
    ]
}

Proposals

  1. Make "main" and "ender" keys accept any of:
    • single file path
    • array of file paths
    • glob matching pattern
    • array of glob matching patterns
  2. Extend "ender" key to allow for the following sub-keys:
    1. "bridge" to represent the bridge file(s) in the same way as a string/array "ender" key would
    2. "dependencies" of the same format as the root "dependencies" key, the existence of ender->dependencies tells Ender to ignore the root key and install the dependencies listed here. A package may be usable in both Node and Ender and have different dependencies for each.
    3. "main" to override the root "main" key where present. Useful for a package that works in both Node and Ender.
  3. Further extend the "ender" key for use with a no-arg ender build in the same directory as the package.json. The descriptor then becomes a build command, replacing all additional ender build arguments. Optional of course, the commandline will still work as normal.
    1. "dependencies" becomes the packages that you would normally specify on the commandline.
    2. "bridge" becomes simply a way to add "noop", any other value is ignored; replacing --noop.
    3. "sandbox" can be either a boolean or an array of package names. A "true" will indicate that the whole build should be sandboxed but that none of the packages should be exposed on window. An array of package names will indicate that each of those packages should be exposed on window; replacing --sandbox [<package>...]
    4. "output" indicates where the build should be written. It should be without a .js suffix but that will be stripped if included anyway. Can include a relative or absolute path or just a local filename (.js and .min.js will come out). Replaces --output.
    5. "compile" will include the additional ender compile step in the build process, running your build through Closure Compiler with any additional files. Can be any of:
      1. a boolean, where "true" will just run Closure over the plain ender.js.
      2. an array of files to include in the build after ender.js.
      3. an object containing:
        1. a "files" key which contains an array of files to include in the build after ender.js.
        2. an "output" key specifying where the resulting compile should go (same rules as ender->output).
        3. an "externs" key which contains an array of files to pass to Closure as --externs arguments.

Examples

1. A package that can be used as both a Node package via npm install and in an Ender build via ender build/add.

"main" descriptors are different for each case, as are Dependencies.

{
    "name": "foo"
  , "main": "node/main.js"
  , "dependencies": {
        "async": "*"
      , "request": "*"
      , "contextify": "*"
    }
  , "ender": {
        "main": "ender/main.js"
      , "bridge": "ender/ender.js"
      , "dependencies:" {
            "bonzo": "*"
          , "reqwest": "*"
        }
    }
}

2. A local package.json that will be read on a plain ender build or ender build <x> where x points to the directory containing this package.json.

{
    "name": "foo"
  , "ender": {
        "dependencies:" {
            "sel": "*"
          , "traversty": "*"
          , "./special/child/package"
        }
      , "sandbox": [ "./special/child/package" ]
      , "output": "./dist/foobar"
      , "compile": {
            "files": [ "./lib/util.js", "./lib/main.js" ]
          , "externs": [ "./res/externs.js" ]
          , "output": "./dist/foobar-app"
        }
    }
}

Would result in the same as the following commands:

$ ender build sel traversty ./special/child/package --sandbox ./special/child/package --output ./dist/foobar
$ ender compile --use ./dist/foobar ./lib/util.js ./lib/main.js --externs ./res/externs.js --output ./dist/foobar-app

You would end up with foobar.js, foobar-min.js and foobar-app.js in your dist/ directory.

Notes

All "main" files will be simply concatenated for inclusion in a build, as long as the build isn't --noop the whole lot will also be wrapped in an exports capturing block. All "ender" files, where present, will then be concatenated for inclusion after the "main" files.

npm is responsible for installing all dependencies from the core package lists all the way down. Introducing an ender->dependencies key means the Ender CLI would have to loop back and install any missing dependencies that npm isn't aware of. Additionally, npm will install unnecessary dependencies from the root "dependencies" key, which would be undesireable--such as when there are lots of unnecessary dependencies and/or dependencies that take lots of extra time to download and/or install (compile perhaps). Ideally we could subvert npm's dependency management process.

--- Want to back this issue? **[Post a bounty on it!](https://www.bountysource.com/issues/742651-proposal-ender-package-json-extensions?utm_campaign=plugin&utm_content=tracker%2F165667&utm_medium=issues&utm_source=github)** We accept bounties via [Bountysource](https://www.bountysource.com/?utm_campaign=plugin&utm_content=tracker%2F165667&utm_medium=issues&utm_source=github).
@iamdustan
Copy link

I swear you just read my mind. For personal clarity does glob matching pattern mean something similar to d./ir/*.js?

How would Proposal 2.3 affect the 4th current use? Main is what would be called in via npm’s require('module') and the ender+dependencies key for usage in the browser side?

Concerning proposal 3—my fork playing with this approach requires redownloading everything from npm every build—even when the version is locked and locally available. Will other changes using npm itself to manage dependencies help resolve this unnecessary roundtrip? What is your opinion of adding an array of objects under the ender key to be able to have a few builds managed? (E.g. core.js with mostly unchanging boilerplate + app.js with the ongoing build of the site)

@rvagg
Copy link
Member Author

rvagg commented Apr 18, 2012

Thanks for the feedback @iamdustan, good to have someone else engaged on this.

Glob matching is just a simple pattern matching mechanism usually used for matching files and directories. It used to be native in Node which was a problem cause Window doesn't have a proper native implementation that matches the *NIX's, but now we have node-glob which uses minimatch internally to translate the patterns to regexes. The basic units of matching would be * for 'any string', ? for 'any single character' and ** (optional) for multiple directory levels (* normally only matches a single dir). So lib/*.js, lib/module?/**/*.js.

Regarding 2.3, there's a error in my original that has "main" overriding "dependencies". The intention is for the ender->main key to override main and ender->dependencies overriding dependencies, where the ender-> key doesn't exist there will be no overriding.

npm bugs me, if you run an npm install on a project with a package.json with dependencies then it'll install them, if you run it again it won't do anything, unless you change your dependencies to introduce something new--it doesn't even check for updates. Run it via the API and you don't get the same effect. This is something I have to look into because I'd rather more closely match the npm behaviour. Over here in AU, I have a latency to the npm repo (and most other US sites) of ~250ms, which is painful where you have dependencies of dependencies (of dependencies...) cause the fetch is synchronous. So this is a pain point I really want to alleviate.

As for an array of "ender" keys, that's a very interesting suggestion. I guess I'd first question the wisdom of it given that Ender is so good at bundling everything into a single file, making multiple files sort of defeats the purpose, but perhaps there's a justification under dev conditions. Perhaps that's just me though.

@rvagg
Copy link
Member Author

rvagg commented Apr 22, 2012

I'm starting to think that overriding dependencies with ender->dependencies perhaps isn't a great idea. The main problem is that we can't stop npm from installing dependencies of any package we ask for, it'll just do it. We could try and get that patched but there are other problems, npm does a build if one is defined in the package, usually builds require dependencies which is one reason they are fairly important. So if we stop dependencies then we stop builds, but perhaps you want a build to take place to get the Ender JS set up?

So there are a couple of options, we could 'override' in the sense that Ender will still get npm to do its normal thing and install the dependencies it wants but then Ender will install any additional dependencies it needs that come from the ender->dependencies key. So you're doubling up. Then, if you install a package that's meant for both server and Ender you'll do a build and fetch dependencies for the server component even if you just want it for Ender; pretty wasteful.

The other option is to not override at all and make the root dependencies key the only way to pull in dependencies for Ender, this would then encourage more of what we have now, that is, packages in npm or client separate from packages for server. So if you have something for server and also Ender then you'd need to publish twice if it was at all complicated, but perhaps that's a good thing.

@michaelsbradleyjr
Copy link

I realize the issues involved are far from trivial, but it seems to me that it would be quite useful to be able to optionally split out dependencies appropriate for an ender-build from those that are appropriate for server-side usage.

A real-world example:

Suppose I'm writing a connector for a database's REST interface (I am actually). For use in the browser, I may want to require iriscouch's browser-request package; for server-side usage, I may instead require mikeal's request package. Those two packages are mostly API compatible, and I can build a common code base around them.

Near the top of my library I'd probably have something like this:

isNodeJS = Boolean process?.pid

if isNodeJS
  request = require 'request'
else
  request = require 'browser-request'

But how to deal with the divergence with respect to ender and package.json? Currently, the best solution seems to be maintaining two separate packages and package.json files. Maybe my library will be published as super-connector (for server-side usage) and I'll also publish super-connector-ender (for use with ender's CLI). N.B. I don't want to maintain two separate code bases per se. The only difference between the two packages would be the divergent dependencies hashes in their package.json files.

Is that ideal, though? It seems like it would be better if I could publish a single package and split apart the dependency groupings within the one package.json.

@rvagg
Copy link
Member Author

rvagg commented Apr 23, 2012

That's precisely one of the use-cases I'm thinking about and it'd be great to handle this nicely. My only issue at the moment, as per my previous comment, is that we can't stop npm from installing request if it's listed as a "dependency" even if it's not needed for the Ender build.

That may not be a big issue for your use-case, but consider socket.io-client which has a dependency tree that looks like this:

└─┬ [email protected] 
  ├─┬ [email protected] 
  │ └── [email protected] 
  ├── [email protected] 
  ├─┬ [email protected] 
  │ ├── [email protected] 
  │ └── [email protected] 
  └── [email protected] 

and ws requires a compile step.

Ideally you should be able to ender build jeesh socket.io-client and have Ender pick out an ender->dependencies key from the package.json that specifies what it's Ender dependencies are (none, I think). But npm is still going to install all of those unnecessary dependencies and compile ws.

But perhaps it's good enough to point these problems out to module authors and let them deal with the flack they'll likely get from this?

@amccollum
Copy link
Contributor

Personally, I don't think installing the node dependencies is a big issue. In fact, given ender's dependence on NPM, I don't see a way around it. If the module is designed to work on both the client and the server, I don't see how NPM could easily distinguish a module in ./node_modules that was installed purely for client-side use in ender, and one that was installed for both client and server use (or server use only). I guess ender could clean up the modules it installs after it builds them, but that would be a big change from the current semantics.

This issue of not being able to specify different dependencies for client/server packages has been really biting me lately, so I'd like to help implement these extensions.

@amccollum
Copy link
Contributor

I'd also like to suggest the possibility of having a name key under the ender key that would override the package name in ender. This would be useful, for example, when creating an ender-compatible version of another library that will provide the same API (similar to what @michaelsbradleyjr is referring to). For example, if I create a browser version of the node events module (which I have), I might want to publish it as ender-events but expose the package name simply as events so that the code can simple require('events').

@rvagg
Copy link
Member Author

rvagg commented Apr 24, 2012

Great to have you on board with this one Andrew. Check out the 1.0-wip branch, I don't think the current master is going to see any significant work now. Unless someone finds it handy for experimentation of course but the new code should be easier to hack if you can get your head around it and keep the tests happy.

@rvagg
Copy link
Member Author

rvagg commented May 9, 2012

as per #57, @coolaj86 points to the "overlay" property in the Modules spec: http://wiki.commonjs.org/wiki/Packages/1.1
perhaps warrants further discussion

@andrewplummer
Copy link
Contributor

+1 for the externs option! Fantastic!

@rvagg
Copy link
Member Author

rvagg commented Jun 1, 2012

A lot of this is done now with the latest batch of 1.0-wip work, released to npm with the tag dev, so you can npm install -g ender@dev to get it. The specific CWD (./) package.json stuff isn't done yet but some of the cool things that work now:

"main", "name", "ender", "dependencies" can all be overridden in any package by including them in the "ender" key of package.json (or alternatively the "overlay"->"ender" key as per the CommonJS Packages spec).

So a package.json can have:
{ "name": "foo", "main": "./node.js", "ender": { "name": "bar", "main": "./ender.js" } }
and you'll end up with a package named "foo" in your build and the ender.js file will be included rather than node.js.

You can also override dependencies in the same way and only the overridden ones will be installed (even though npm will also install the root dependencies--they are just ignored in your build).

I'm still missing some functional tests to lock the changes in, plenty of unit tests for this stuff though so it's pretty solid.

Also notable in this release is that Ender won't fetch packages from npm that are already installed in node_modules unless you use the --force-install option, I've tried to collect the changes in Changelog.

@michaelsbradleyjr
Copy link

Sounds fantastic! Thanks for all your hard work.

@coolaj86
Copy link
Contributor

coolaj86 commented Jun 1, 2012

How about instead of ender the key be named browser?

onejs and pakmanager both do part of and part more than what ender is doing and it would benefit us all to use a common overlay system for browser packages.

@coolaj86
Copy link
Contributor

coolaj86 commented Jun 1, 2012

For pakmanager to use browserDependencies and ender to use ender and onejs to use... whatever they use... just no good for the community at large.

And I believe it would be better to publicly support a single standard (i.e. overlay.browser) rather than two (browser and overlay.browser), but still support the accidental usage for backwards compatibility.

@rvagg
Copy link
Member Author

rvagg commented Jun 1, 2012

I'm happy to adopt appropriate aliases if you think that's necessary but I think I'd rather keep the "ender" key because there will be a bunch of very Ender-specific stuff to go in there. The logic for this is done in one place only so it's pretty easy to amend. Perhaps you'd like to put up a PR for discussion?

@iamdustan
Copy link

I love you.

@rvagg
Copy link
Member Author

rvagg commented Jun 1, 2012

😊

@amccollum
Copy link
Contributor

In the original proposal, the bridge would be overridden with a bridge key under the ender key, but in the source, it looks like it is done with ender.ender. Is this the way it will work going forward, or can I implement the originally described behavior?

@andrewplummer
Copy link
Contributor

I'll freaking love you hard if that externs thing happens to resolve itself. O:)

@ded
Copy link
Member

ded commented Jun 1, 2012

whoa.

@rvagg
Copy link
Member Author

rvagg commented Jun 1, 2012

@amccollum I still want to go with the "bridge" key but it's a special case I haven't got to yet. I think it'll just be an alias for "ender" so they will both work.

@amccollum
Copy link
Contributor

Well, I submitted a pull request (#147) to make this work (just with the bridge key, but it's one more line to add ender, too).

@rvagg
Copy link
Member Author

rvagg commented Jun 1, 2012

Cool. Sorry, didn't notice the PR. Will take a look ASAP, thanks!

@rvagg
Copy link
Member Author

rvagg commented Oct 15, 2012

@andrewplummer that "externs" thing is in and I'd love for you to test it is you're still inclined (I know it's been a while!).

The package.json "externs" key of any bundled lib can be either a string or an array of strings pointing to files within the current package, relative to the root. These will be passed, relativised to the CWD, to Closure when you're doing a --minifier closure build (you can supply additional externs with the --externs <file> command line argument).

Because overriding is now in action, your "externs" could be on the "ender" subkey or even "overlay"->"ender" and it'll have the same effect.

I have unit tests for internal functionality but functional tests are a bit trickier so I'd appreciate it if you were able to test this (with Sugar I guess?). A package in npm that has a valid & working "externs" would go a long way towards a functional test for this.

You'll need the 1.0-wip branch, either clone the repo or npm install -g ender@dev to get the latest.

@andrewplummer
Copy link
Contributor

Cool! Will do!

@iamdustan
Copy link

@rvagg and to think you’ve been so quite lately and then you come out with this. Word.

@andrewplummer
Copy link
Contributor

Hey... I had a bash at it but didn't have any luck... not sure the version is right though. I cloned the repo and set to 1.0-wip branch, then installed with -g flag... first I got this:

Child Process Error: Command failed: Unable to access jarfile /usr/lib/node_modules/ender/support/closure.jar

So on a whim I copied my version of the compiler to that file then compiled again. Compiled, but the externs weren't applied apparently. However, running ender -v still says 0.9.5, so I don't know if it's an older version or what... Also I can't see if the --externs flag is being passed to the compiler or not, also I don't know if the compiler fails silently if the externs file can't be found or what. Running compiler --debug doesn't seem to show me any extra info...

@rvagg
Copy link
Member Author

rvagg commented Oct 16, 2012

I should have mentioned that this is now working on ender build, not ender compile.. I'm still in two minds about compile and I'm inclined to fully phase it out, replacing all the functionality with build options. The version is correct, 0.9.6-dev is the latest that I've just published with the @dev tag. If you have the repo then pull the latest 1.0-wip and update the dependencies as I've also updated the ender-minify package.

When you run ender build sugar ... --minifier closure then it'll pull externs out of all packages that are includes (including dependencies). It should also take the --externs <file> argument for additional externs.

If you want to see the actual Closure command then edit node_modules/ender-minify/lib/closure.js and console.log(javaCmd) at about line 62.

Since we're not reading anything from dependencies in ender compile it's not as simple as the ender build case where we're reading them anyway so we can pull out the externs. The main thing missing that would make ender build able to do what ender compile does is the inclusion of individual .js files, but @amccollum has just started working on that.

@andrewplummer
Copy link
Contributor

Oh... well if that's the case, then I think it's already working... I tried loading the ender.min.js file and was having no problems with it. To be honest, I can't say with 100% certainty that it wasn't always working... my problem had always been with ender-app.js and compile. However it's also true that Sugar has gone through a lot of changes itself, not the least significant of which is that all packages are now being compiled into a single file which is now included in main in the package.json, which might also have an effect.

Unfortunately (for testing this anyway), ender build also seems to be working without the externs parameter in the first place...

@andrewplummer
Copy link
Contributor

Ah wait... I just flashed on something... are you saying that if you don't specify --minifier closure then it won't use closure? Is it minifying with something else like Uglify?

@rvagg
Copy link
Member Author

rvagg commented Oct 16, 2012

Yes, UglifyJS is the default

@andrewplummer
Copy link
Contributor

Ahh ok...well that would do it :)

Well funny... I build it and even without the externs file it seems to work... It's good news but I'm still not sure what to make of it ....

@amccollum
Copy link
Contributor

I'd like to propose that ender->dependencies be required to be a subset of the main package dependencies. The primary reason for this is to simplify the internal package install process so that it can occur in a single pass (with NPM resolving the dependency graph alone rather than splitting the work with Ender). The result of this change is that it will simplify the code base, make it more modularizable, and make it easier to extend Ender to work with other package managers (e.g., Bower).

The main argument against this proposal is that it requires some repetition and additional verbosity in the package.json file. There isn't really a difference in speed since we install all the same modules either way.

Of course, Ender can emit an error if it finds that this rule is being broken.

Do others have thoughts about the benefits or drawbacks of enforcing this requirement?

@rvagg
Copy link
Member Author

rvagg commented Nov 5, 2012

I'm fine with this btw, it has added a fair bit of complexity to support a totally separate list and reducing complexity is always a good thing.

@mysticflute
Copy link

This is really good stuff. If anyone else is also waiting for the ability to specify the output location in package.json, and you also use Grunt, I created a Grunt plugin to help with this problem (until the real solution is in place).
https://npmjs.org/package/grunt-ender

@lisp-ceo
Copy link

+1 for grunt-ender

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

9 participants