Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add files[] for new ender compat to require relative files #223

Merged
merged 2 commits into from
Nov 9, 2013
Merged

Conversation

ded
Copy link
Contributor

@ded ded commented Nov 7, 2013

hi there,
in love with your promises implementation and it's heavily in use over here at Change.org. We have a node application which purposes its modules on both server and client. We build our client packages with Ender and one of the caveats of using when is that we can't require relative modules to the package. Eg: require('when/parallel')

We are about to cut a release of Ender@2.0.0 which solves this issue but requires package maintainers to list the files which can be included.

Sorry in advance if this is a PitA.

@unscriptable
Copy link
Member

Everybody wants a piece of our package.json files these days! :)

Hey Dustin!

To my AMD+CommonJS brain, this kind of thing seems unnecessary since the build tools and run-time environment are able to find relative modules very easily. Obviously, I don't know enough about ender to know why it's different. Perhaps a simpler mechanism is the right choice in many situations. If that's the case with ender, then that's cool. :)

We've accommodated package.json entries for other packaging/build systems in the past (i.e. volo in curl.js). In short, we're certainly not opposed to this. It's @briancavalier 's call wrt when.js. He's the lead on this project.

I have a concern, though. Have you been following the bower.json discussions about fine-grained file inclusion for build tools? It seems confoundingly similar, but incompatible. :(

bower/bower#935

{
    "main": "myLib",
    "files": { 
        "scripts": ["foo.js", "bar.js"],
        "css": ["dark.css"]
    }
}

Their proposed "files" would be for non-modular scripts, not modules and it would be a hash of file types. Modules don't need special treatment since they are assumed to be consumable by a module-aware environment.

Of course, bower uses bower.json, not package.json so there's no immediate incompatibility. Still, I fear that as an industry, we're flailing, scatterbrained, and diverging when we should be converging.

Why can't we all just work together?

@amccollum
Copy link

The array format is to be compatible with the npm package.json spec. Honestly, though, it's not exactly incompatible since (if Bower decides to implement the object-based spec) Ender will probably just recognize whether the field is an array or an object and act accordingly.

Obviously, if you do just define files at the top level (rather than under the ender subkey), Ender will read that just fine, too.

@briancavalier
Copy link
Member

Thanks @ded! It'd be great to provide Ender support. Since we can namespace this under an ender key, it doesn't bother me (too much :) ) to add it to package.json.

One worry is that this creates a maintenance task of ensuring that the ender.files array is kept up to date. We don't add new js files very often, but that might be precisely why this could be a problem … i.e. updating ender.files never becomes a habit, so we'll forget.

Another is that if every package manager wants it's own variation of a file list (bower, component, Ender, etc.), then maintenance headache just gets multiplied. I agree with @unscriptable here that we're in the "flailing" stage of package management. I'm sure the community will converge eventually, but right now it's diverging.

I say we move forward with this and find a way to mitigate the "I keep forgetting to add new files to ender.files" problem. Are there any tools that will do that automatically, like maybe a Grunt task? We're already considering adding a build step for legacy browser global support, so if we could automate the ender.files update at the same time, that'd be ideal. Ideas?

@amccollum
Copy link

Ender supports glob syntax, as well, so if deprecated/support files were moved to a different folder, that might simplify the maintenance task. Something like:

  "ender": { "files": ["*.js", "node/*.js", "unfold/*.js", "monitor/*.js"] },

@briancavalier
Copy link
Member

@amccollum Ah! Globbing def eases the burden a bit. Ideally we'd still automate, but this helps. Thanks.

@ded
Copy link
Contributor Author

ded commented Nov 8, 2013

@unscriptable @briancavalier yes to be fair, i'm fully aware that everybody wants a piece of our package.json — which is why i was reluctant at first. Trust me, I played the AMD/CJS/vanilla voodoo game quite a few times and that becomes a pain too.

@briancavalier I can update the PR to use glob syntax or allow you to do it yourself. fwiw, you know your repo better than I do. Obviously I selfishly started with when because it's high on my list of fav modules.

@amccollum thanks for chiming in

@briancavalier
Copy link
Member

@ded Cool, yeah, please switch to globs, and we can get this merged fairly soon. The globset that @amccollum proposed looks right … not sure if ender supports the slightly more compact "files": ["*.js", "[node,unfold,monitor]/*.js"] glob syntax, but whichever works is fine w/me.

@briancavalier
Copy link
Member

@ded Also, you can ignore Travis failures here. Our travis setup currently has a problem with pull requests. Hopefully, we'll have that fixed sometime soon.

@ded
Copy link
Contributor Author

ded commented Nov 8, 2013

there it is.

briancavalier added a commit that referenced this pull request Nov 9, 2013
Add ender 2.0 support to package.json
@briancavalier briancavalier merged commit 3c7abdf into cujojs:master Nov 9, 2013
@ded
Copy link
Contributor Author

ded commented Nov 10, 2013

awesome

@ded
Copy link
Contributor Author

ded commented Nov 10, 2013

looks like we messed up the glob, but i think it's a bug we need to fix on the ender side. given that *.js picks up when.js, it gets duplicated in the built file since when.js is already the main. // @amccollum

@briancavalier
Copy link
Member

Thanks for the heads up. I'll wait to hear how you decide to handle this on the ender side. If we need to tweak the glob in the short term to get this released, just let me know.

@amccollum
Copy link

Yeah, I have a fix for this locally, but it's wrapped up with a bunch of other changes. I should push it out today or tomorrow, though. The globs should be fine as written.

@briancavalier
Copy link
Member

Sounds good.

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

Successfully merging this pull request may close these issues.

4 participants