Add support for npm (package.json) #41

Closed
gobwas opened this Issue Apr 23, 2014 · 13 comments

Comments

Projects
None yet
6 participants

gobwas commented Apr 23, 2014

Am I wrong, or there is no way not to install it via npm?

Why would you want to install it via npm if it's a frontend library?

gobwas commented Jun 17, 2014

And? =)
My project uses browserify and npm.

Anyway, CJS module definition could be useful.

Oh, nevermind, I don't know how Browserify works yet.

+1

Collaborator

axelpale commented May 3, 2015

Totally +1. This should be a de facto feature of all modern js libraries. I'm almost frightened to discover move.js is not in npm. Without this it is hard to browserify. Scandalous!

silvenon commented May 3, 2015

Not really. But it would be nice to have it on npm.

Collaborator

axelpale commented May 3, 2015

This should be easy. Four of the five dependencies have package.json and are already in NPM:

For the fifth dependency, css-ease, I already created a pull request providing package.json and the NPM support.

After that a package.json can be created for Move.js and npm publish be run.

There is one issue though. Fortunately it is easily solved. The index.js uses require('emitter') and require('query') instead of require('component-emitter') and require('component-query') required by NPM. Event they are the same modules, their names are different. In component.json GitHub URLs are used thus enabling the shorter names. Fortunately these same GitHub URLs work in package.json as long as the repos of the URLs have their own package.jsons. Issue solved.

I truly hope that this inclusion of package.json and publishing to NPM is supported by the Move.js community. It is hard to me to see any downsides.

@silvenon, thanks for the tip regarding browserify-shim :)

silvenon commented May 3, 2015

Wow, you were much more helpful, nice 👍

Collaborator

axelpale commented May 4, 2015

Pull request incoming. Before that I want mention some additional issues that were on the way.

First, thanks to @yields for publishing css-ease to npm with such a short notice.

The issue I mentioned in the previous comment turned out harder than first expected. The require('emitter') and require('query') threw errors about missing modules even when the GitHub URLs were used in package.json. This was understandable because component-emitter and component-query truly are the identities of the modules, defined in their package.json files, and therefore in spite of many approaches I tried, npm install stubbornly included the modules as such.

At first I desperately searched some kind of module aliasing feature in component.json or package.json. Of course issue would be solved if component-emitter could be aliased with emitter and same for component-query or vice versa. As it turned out, no aliasing feature exists and for a good reason. It would break some primary design principles of both components and npm modules as discussed by @tj and others in here, here, here and here. The issue suddenly seemed to have very deep roots in the incompatibility between component(1) components and npm modules.

A try/catch solution were hinted by tj and proposed by stephenmathieson to fix the issue. An ugly one but seemed to be the only option. I tried it and it worked out but while analysing the resulting move.js script, I surprisingly stumbled upon something much more elegant: component(1) understood both require('emitter') and require('component-emitter'). For component(1) they both are valid ways to require the component listed as component/emitter in the component.json. Either require('emitter') or require('component-emitter'), component(1) translates both to require('component~emitter@1.2.0'). Superb.

So no more tries or catches were needed. In the end, the only thing needed for package.json compatibility was to change require('emitter') to require('component-emitter') and require('query') to require('component-query'). Both component(1) and browserify understand them; peace has returned to the valley.

Collaborator

axelpale commented May 4, 2015

The pull request has now been submitted and waiting for the merge. Could @yields help us out here, once again? :)

Hey @yields - any change of merging this and publishing to npm? 👍

Contributor

yields commented Jun 23, 2015

i can't maintain this lib anymore :(, don't have time to invest in it ATM (cc @tj)

Collaborator

axelpale commented Nov 30, 2015

Finally, released to npm \o/.

@axelpale axelpale closed this Nov 30, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment