New way to handle packaging #181
Comments
|
just about to hit the sack for the evening until I read all these changed. I think the direction is pretty awesome given the stated goals. internal requires has been a feature even I have wanted for a long time. I think one thing that should remain a goal is to keep the client interface, even if it isn't the default build (eg: the jQuery syntax). Also, quite a number of popular modules (with old bridges) would have to get updated build processes |
|
Yeah, to be clear, I definitely think the client-interface is integral to Ender, even if technically it can be replaced. Part of what makes Ender great is that it is opinionated about a small number of core decisions, so other modules can build on those common assumptions. Another example of this behavior is that even with the new Module class, we auto-require every module (and bridge file) once on startup, in case modules modify the global context or need to do other initialization. While this isn't strictly following how Node behaves (you have to require a module once before its code runs), I think it's pretty important for striking a compromise for convenience in a browser environment. I'm not sure how many modules use the array convenience for their |
|
would be great to know! As long as none are mine 😉 |
|
More control of the build is awesome, especially if it could make an easier road to getting source maps implemented. |
|
Repeating my comment here for clarity: IMO: at this stage, since we mostly seem to be time-poor, he who does the work, gets to decide on what gets done, so over to you! Thanks for picking up some of the slack. @amccollum if you have a vision for how this should work then I'm behind you going for it, my vision is limited at the moment because I'm stretched across too many things. |
|
i tried reading thru each of the commits. does this finally give us support to include local files in packages....eg: var parallel = require('when/parallel')
parallel([a, b, c]).then(done) |
|
Yeah, it does. It supports both relative includes within a package and cross-package submodule includes. On Tue, Aug 13, 2013 at 7:53 PM, Dustin Diaz [email protected]
|
|
it would be great to have these branches published to npm as |
|
Ok, I think this branch is ready for others to try out. I had to remove some tests that were obsoleted by these changes, but there is currently only one test failing (a minifier test that seems unrelated, but I need to keep trying to track down the cause). I also added tests for the parts of A new regression in this build is the removal of the I've updated all the modified modules to have |
|
@rvagg can you |
|
done, and all the associated packages too, let me know if I've missed anything @amccollum, when you publish just remember to publish with |
|
oh, and I think you might have to pin exact versions across the packages otherwise npm will try and match |
|
Ok, I published this version under |
|
@amccollum @rvagg k. i'll have a look now given that i was pretty excited about some of the new features you introduced... |
|
I'm actually about to push some other improvements I've been working on that required a more major refactoring. For example, Bower support as the repository backend. On Thu, Nov 7, 2013 at 1:52 PM, Dustin Diaz [email protected]
|
|
@amccollum one thing i was looking for was |
|
Yeah, so the one change that a module owner has to make to have this work as expected is to add a lets you do |
|
so unless i was missing something (obviously). i did a simple edit: oh, just seeing your new comment now |
|
ok, i went through and looked at all of it. i see you even kept support for i say we move forward with the big |
|
I'd like to be given permissions to create new repos under the Not having package handling code sprinkled throughout different modules paves the way for adding support for Once I get the permissions for the organization, I'll push the full set of changes. I'd appreciate some help testing with the new handling of packages on the CLI side. I still need to add some tests for the new dependency mapping stuff, but otherwise the code and tests are in pretty good shape. I'll also add all the other owners to the new packages in npm. |
|
Is there any reason npm can't be used for the back end? Bower is just a front end for github, which npm handles. |
|
yeah, the whole Bower thing... I just don't understand it. But anyway, yeah, I had a play with github urls too and that worked out nicely. It looks like you also have a patch for the duplicate sub-include that i mentioned. It would be great to put that in as well. Other than that, I'll set up each person with admin access. ... oh, and last but not least, it would probably be a good idea to re-think our licensing/contributor notes. I am more inclined to simply include a short URL linking to our copyright with also includes the proper list of contributors |
|
I guess I don't really understand the appeal of Bower, either, but I imagine it will be attractive to some people to have that option. Obviously NPM should remain the unequivocal default. |
|
@amccollum where we at with this? I can begin working on new docs |
|
Sorry, got a little busy the last couple days. As soon as NPM comes back up, I should be able to run through all the tests one last time and then push/publish everything. |
|
one other thing if you wouldn't mind. Add some examples of the new |
|
Ok, the I'll start working on examples, and adding some final test coverage. A few other things worth noting:
|
|
fwiw, i may have been the only one using |
|
@amccollum i just checked |
|
also closes #120 |
|
Yeah, nothing doing on adding you guys as owner, NPM just keeps erroring when I try... I'll create an issue there. |
|
@amccollum I work on npm, what's the command/output? |
|
|
@amccollum you might be using the wrong order of commands, are you doing |
|
It's confusing. |
|
@luk- That's embarrassing. It's been a long day. Thanks. |
|
It's cool, I work on npm full time and always have to check what it is, but I'm also kind of an idiot. |
|
@luk- ok, well, I added you guys (again), but every so often I get this when I run the command: NPM has randomly been saying packages are either not in the registry, or versions are overwriting nonexistent version all day... |
|
@amccollum I'm getting the 404 on packages occasionally as well. This stuff should clear up in the next 24 hours, Unfortunately, I don't think there's really anything we can do about it in the meantime. |
|
If it doesn't work tomorrow I can get someone on the server side to take care of it for us. |
|
It seems to be showing up correctly for now, so as long as it sticks, I think we're ok. |
|
rock on |
|
quick question. what is the new |
|
It indicates a core package that doesn't get wrapped in |
|
so basically |
|
Right now, that's correct. Presumably if you were going to replace |
|
@amccollum not sure what happened with the latest... but |
|
try throwing this in a folder {
"name": "example"
, "version": "0.0.1"
, "private": true
, "dependencies": {
"when": "cujojs/when"
}
}
then |
|
Yeah, looking at this now. There are two issues that make this fail right now. The first has to do with the way dependencies are converted into packages ids. Currently, the code just does The second problem, though, is that the current dependency satisfaction code doesn't know how to figure out whether a dependency like this is met or not. To do that, it will need to record the actual name/version that is installed. This gets complicated since we install dependencies in batches. Let me play with this, though. |
|
fwiw, it was working in the i've since tried adding logs and what not... and now they're stuck in |
|
Ah, so |
|
i think an appropriate GH url is |
|
I think the solution here is going to be to keep track of a mapping from installed ids to packages on disk, so when we look for something that has been installed, we know how to find it. |
|
anyway, i can't seem to use |
|
Ok, I pushed a set of changes to ender-js/ender-repository@2e60c5d |
|
i got this working now. given i've been using <script src='ender.js'></script>
<script src='example.js'></script>var when = require('when')
var a = function a() {
console.log('a')
return when.resolve('hello')
}
var b = function b() {
console.log('two')
return when.promise(function (res) {
setTimeout(function() {
res('world')
}, 100)
})
}
var c = function c() {
console.log('three')
return 'almost'
}
var parallel = require('when/parallel')
parallel([a, b, c]).then(function (a) {
console.log(a)
})I get no logs in the browser, but when run in node... cc // @cujojs |
|
alright... i'm actually going crazy and probably missing how JavaScript is working. I know this is besides this issue, but I may as well ask how the hell this works... function parallel(tasks /*, args... */) {
return when.all(Array.prototype.slice.call(arguments, 1)).then(function(args) {
return when.map(tasks, function(task) {
return task.apply(null, args);
});
});
};wouldn't |
|
Yeah, oddly, it is. |
Ok, so I've updated my previous changeset that implements a new
Moduleclass at runtime. The changes fall across 7 different modules:Each of the above has a
new-packagingbranch that includes the changes. The changes in most of the modules are fairly minor, with the lion's share of the changes coming in theender-builderpackage in ender-js/ender-builder@4439aeeThat commit is huge, so I've tried to comment on some of the larger changes inline to explain what's going on.
It's also worth looking at the new ender-commonjs module, which defines the new way packages are created using a
Moduleclass at runtime.Here's a short summary what these changes bring:
Moduleobject at runtime, supporting multi-file modules, internal/relative requires, requiring submodules, and better CommonJS compatibilitymainorbridge, the fix being to either use internal requires to mimic this behavior, or force packages to do their own concatenationI updated all the current tests to work with these changes, but I haven't gotten around to writing tests for the new
assemble.jsfile, yet. This leaves some holes in the test coverage, mainly around the specific content of the final build. It may take me a little while to get around to this, but I wanted to get the rest of the changes out for comment while I finished up the tests. There may be other bugs as well.@rvagg, I'd love to hear your thoughts in particular, because I know you were a bit skeptical of some of these changes when I submitted them before.
--- Want to back this issue? **[Post a bounty on it!](https://www.bountysource.com/issues/969742-new-way-to-handle-packaging?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).The text was updated successfully, but these errors were encountered: