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

CommonJS proposal #298

Closed
maccman opened this issue Feb 23, 2012 · 30 comments
Closed

CommonJS proposal #298

maccman opened this issue Feb 23, 2012 · 30 comments

Comments

@maccman
Copy link

maccman commented Feb 23, 2012

I think it would be awesome if Sprockets supported CommonJS, and I think you could implement it in a backwards compatible approach.

I've been spiking CommonJS support for Sprockets in a branch:
maccman@9e0e82d

For developers, the end result is:

  1. Signify a file is a CommonJS module with the '.module' extension, i.e. 'myfile.module.js'
  2. Include the 'commonjs' lib somewhere, i.e. #= require commonjs
  3. Start requiring modules, i.e. require('my_module');

There's a number of issues with my initial approach - such as not parsing out requires, having no dependency tree etc. I'm planning on resolving all this on the weekend.

However, my question is - if I did this work, is it likely to get accepted?

@stuartlynn
Copy link

+1

@josh
Copy link
Contributor

josh commented Feb 23, 2012

I'm interested and would like to see this solved eventually.

I do like the stitch approach which this is similar too, but I know people are going to be upset that its not the same AMD standard or whatever. There isn't much of a standard for publishing these types of packages. And I don't want to introduce a new require variant into the JS ecosystem. Shipping JS as rubygems still bugs me a bit.

I'm tracking jquery's new plugin effort. I think it would be interesting to support their package.json standard and handle all the dependency resolving. Its still going to be a while before they shake things out.

Is there anyway you can ship this as a plugin for the time being? Do you need any extra hooks?

@maccman
Copy link
Author

maccman commented Feb 23, 2012

Sure thing, I'll implement it as a plugin for now :)

@josh
Copy link
Contributor

josh commented Feb 23, 2012

@maccman I'm happy to see someone putting in the effort into this. Just know this won't get solved over night. You're going to need to have some patience with me since its not my top priority right now :)

I will keep this issue open for future discussion.

Note For anyone following along, any "+1"s aren't going to add any value to this discussion. I know people would like to see this. Please only comment if you have relavent details to add.

@searls
Copy link

searls commented Feb 23, 2012

+1, this is a cool idea. I generally agree with @josh -- it'll be a while before the dust settles, and would probably confuse folks if Sprockets included it, but a sprockets plugin that got interested folks started with CommonJS modules could only be a good thing. I'd be happy to contribute

@erithmetic
Copy link

I've already started down this path with a Rails plugin, http://github.com/dkastner/nodeify

I went with an existing project called browserify to do the require parsing. There are several advantages to doing this:

  1. The require() parser in browserify is already written and works well (no wheel reinvention)
  2. You can use the cornucopia that is npm instead of relying on JS rubygems or one of the dozens of other JS package managers that will likely be abandoned.
  3. With node and npm, you are instantly blessed with a great command-line testing environment (see buster.js and vows.js).

While working on this I decided that the sprockets //= require() didn't really fit well with the CommonJS style, especially when moving your testing into the command line, so I gave up on backwards compatibility with those directives. Principally, this arises when you might //= require('jquery') in your app, but when testing from the command line, you're stuck having to do some gymnastics during test setup that emulates the DOM and then loads jQuery globally. Luckily, there is a jquery npm package that makes it a little more well behaved in the CommonJS world and using a CommonJS var $ = require('jquery); works well.

The only downsides so far:

  1. Requires node installation
  2. I'm still waiting for a patch to browserify to be accepted to allow other JS files to be required outside of the main JS file's root (this is key for rails projects where I might require something in app/assets/javascript/main.js that is in lib/assets/javascripts/other.js.

If any of this interests you at all, I'd love feedback/help! I wanted to throw this out there in case it could save you time.

If you'd like an example of an app that I've started to port to nodeify, check out https://github.com/brighterplanet/hootroot1/blob/nodeify/app/assets/javascripts/application.js

@erithmetic
Copy link

One more thing: another advantage of node/npm/browserify is that you can have a package.json in your project that acts similarly to a Gemfile, making JS package management just as easy as rubygems.

@maccman
Copy link
Author

maccman commented Feb 23, 2012

@josh I've got a basic version working here:

https://github.com/maccman/sprockets-commonjs

It uses a library I've written called Holmes to find calls in 'require()' in the JavaScript AST. Unfortunately this library requires node - however I can't really see a way round this apart from parsing the AST in Ruby.

https://github.com/maccman/holmes

Anyway, have a few Sprockets questions:

  1. It looks like modules are only being picked up if they're called myfile.module rather than myfile.module.js. Is there anything I can do fix this?
  2. I'm struggling to inject the commonjs.js asset, as if you give an absolute path to require_asset, it complains that the file's not in it's default directories to watch. I can't see a straightforward way of appending to that paths array either. Anyway to get round this?

Cheers,
Alex

@josh
Copy link
Contributor

josh commented Feb 23, 2012

@maccman

  1. Hmm, engine extensions are processed right to left. The left most being an optional mime type. foo.js.coffee.module.erb would run ERB, then Module, and Coffee last. I don't know of an easy way to flip the mime type and engine without totally messing everything up :) The easiest hack would be something like .commonjs. That sucks though.
  2. Right, its gotta be in a path so it can be relative to something. Theres no global path hook, but I'm not sure I want to add something like that. For now, I'd recommend telling people to append_path Sprockets::CommonJS.path.

Statically parsing the AST is scary. Theres no way you can handle cases like require("backends/" + backend); I think the closure lib does something similar, but they enforce all requires to be at the top level and only require with string literals. So they get away with a simple regexp.

I know your researching commonjs more for application purposes. I've mainly been thinking about the "package" usecase.

Assets.append_path "vendor/packages"

vendor/packages/
├── backbone
│   ├── backbone.js
│   └── package.json
└── jquery
    ├── jquery.js
    └── package.json

We could use a similar hack to index.js. If you require a directory, but it has a package.json, treat it special.

backbones's package.json would specifiy it depends on jquery, so you can assume that require. However, these are simple single file cases. I dunno how you'd adapt node's runtime relative requires.

I was thinking about all these stuff for stitch, but then node's package.json structure became less defined and they removed the idea of a load path. Which totally crushed my dreams.

@sstephenson
Copy link
Owner

Thanks for starting the investigation. I'd love to see some sort of runtime module exposure in Sprockets.

I think AST parsing is a non-starter. It immediately excludes a whole class of dynamic programming techniques like @josh mentioned above. These aren't just theoretical use cases either—we used them when we built Basecamp Mobile for things like automatically loading templates in a view class:

template = require "#{@app.name}/templates/#{path}.#{@type}"

Maybe a simpler way to start with modules would be with a simple directive in each file:

#= module

This would automatically wrap the file in a closure in the asset bundle, keyed in an object by its logical path and exposed to a require function.

@sstephenson
Copy link
Owner

It's worth noting that Sprockets' JST support is already a sort of runtime module system.

@maccman
Copy link
Author

maccman commented Feb 23, 2012

@sstephenson I agree that there are lots of issues with AST parsing, such as dynamic calls and requiring Node. At the moment, for dynamic calls, I'm issuing a Ruby warning. The thinking was that if you wanted to do dynamic requires, you should explicitly require all the files as well, either through require(), or #= require_tree. I have a feeling this is an edge case though.

I'm in two minds about AST parsing as well - but it seems a bit Janky to have to duplicate all requires (i.e. in the conditional comments, and using require()). I think this approach is a good compromise. The fact that it requires Node though, could be a deal breaker.

Isn't having #= module support basically the same as having .module.js in the file ext?

@sstephenson
Copy link
Owner

Yeah, a module directive is no better than an extension. My bad.

I agree that duplicating requires is nasty in general. But it's less offensive when you think of it as bundle inclusion (require directive) vs. runtime module access.

It gets simpler if we assume most people will want to pull in all modules into their bundles with one of the bulk-require directives like require_tree. A third-party package foo can consist of many modules as files internally and export them using a manifest file that simply does #= require_tree .. Then a single #= require foo in your asset bundle gives you runtime access to all the modules under the foo/* namespace.

@josh
Copy link
Contributor

josh commented Feb 23, 2012

@maccman since you're the author of spine, I'm curious how you see that fitting into this system.

@maccman
Copy link
Author

maccman commented Feb 23, 2012

Yes, I think you guys may be right. I'll remove the AST stuff.

@maccman
Copy link
Author

maccman commented Feb 23, 2012

Yes, I guess Spine's spine-rails gem would have a require_tree in spine.js, pulling in all of it's sub resources. You could then call require('spine'). People without the sprockets-commonjs gem would just get the global Spine to use. Pity we can't use npm for all of this stuff though...

@quackingduck
Copy link

(subscribe)

@maccman
Copy link
Author

maccman commented Feb 24, 2012

Ok, it's done now - what do you think?

https://github.com/maccman/sprockets-commonjs

@metaskills
Copy link

Thanks @maccman, I will definitely put this on my list of things to test in my current Rails/Spine.JS application.

@maccman
Copy link
Author

maccman commented Mar 4, 2012

@josh, @sstephenson what do you think of the proposal? Any reason this couldn't be included in core down the road?

@andrewdeandrade
Copy link

I just read this thread from top to bottom because I'm interested in solution that permits me to use common.js with rails. Way up at the top @dkastner offered an alternative solution that near as I can tell appears very viable and moves as close as possible to a pure javascript approach to handling javascript assets. However I didn't see any comment from anyone else regarding @dkastner's proposal.

@josh @maccman @sstephenson Since you three seem to be most involved in the discussion and have deeply explored ruby-based, I'm especially curious to hear you views on @dkastner's approach, especially since at the end of the day both solutions require node.js anyways and it looks like his approach would allow us to avoid have to use both require() statements and sprockets directives.

@maccman
Copy link
Author

maccman commented May 7, 2012

@malandrew afaik only parsing the AST requires Node - the other approach doesn't. Parsing the AST is really slow too.

I'm quite happy doubling up require calls, as I find most of the time I'm using require_tree anyway.

@andrewdeandrade
Copy link

@maccman so when you removed AST parsing from sprockets-commonjs, you also ended up removing the node.js dependency from the lib? If so, does removing node.js as a dependency matter that much because AFAIK you still end up with v8 as a dependency via therubyracer in order to perform uglification/minification?

When you use require_tree, do you basically use it for everything, but that final javascript onready/onload module that kicks off the execution of your single-page app, or are there other places where you explicitly link to a file. I might be naive here, but as the common.js approach wraps modules up using function declaration, it seems like file order doesn't really matter at all, does it?

Any other views on @dkastner's approach versus yours. I'm looking at both and trying to figure out which one is best for our use. Are you already using sprockets-commonjs in production?

@maccman
Copy link
Author

maccman commented May 7, 2012

@malandrew That's a good point. However, the uglify gem actually relies on execjs - which doesn't require v8 compilation (although still requires some sort of JS runtime). Due to the way Holmes is implemented, it specifically relies on Node (which breaks execjs somewhat).

As for require_tree, I basically use it for everything (i.e. the app dir). That's correct, file order doesn't matter. The only other thing I'd say is that I've been down a number of paths trying to solve this issue - and imo the simplest solution is the best here. Yes, we're using sprockets-commonjs in production at Twitter.

@andrewdeandrade
Copy link

@maccman Awesome that you are already using it in production at Twitter. Looks like when I'm done building this prototype for a potential client, I'll go ahead and implement sprockets_commonjs in my startup's app.

Personally, I'm still interested in a solution like @dkastner's because we're looking to move to node.js long term (at least for real-time and event-driven parts of our API) and I personally would like to work towards a complete decoupling of our client-side app and API backend. Moving to sprockets-commonjs and then to nodeify would give a path to a complete separation of concerns.

So if I got this right, you no longer use Holmes in sprockets-commonjs, correct?

Don't you think being able to leverage node and npm would be more valuable long-term for handling all javascript assets, or would that be out of line with there Rails is trying to position itself? It seems like an issue of where the complexity lies. The nodeify approach reduces complexity for those largely responsible for building the javascript front-end (the assumption here is that it is large, which is a reasonable assumption if commonjs is seen as warranted) at the cost of complexity for those managing the production environment. Does requiring node.js and npm add that much more complexity to the production environment? As someone who I believe is primarily a javascripter, I figured you'd favor an approach that leverages npm.

@jimmycuadra
Copy link

Keep in mind, when considering the long-term usefulness of this, that ES.next will have its own module syntax which does not use CommonJS. Dave Herman says the plan is for Node to migrate to this new system once it is implemented in V8.

@andrewdeandrade
Copy link

@jimmycuadra True. The way I see it any step from sprockets directives towards common.js is already a step towards organizing your code in a way that switching to ES.next should be simpler if upgrading your node.js version in the future requires updating how you declare dependencies. I also reckon there will be ways to maintain backwards compatibility with commonJS for sometime afterwards as well.. Right now we're using namespacing in our app and it is painful.

@erithmetic
Copy link

@maccman the speed penalty of walking the AST isn't really an issue if you precompile your assets for production. It just slows down your page loads in development. I've seen about 2s of initial load time on a fairly complex (w.r.t. module require depth) site (http://hootroot.com, http://github.com/brighterplanet/hootroot1) on my old white MacBook.

@josh
Copy link
Contributor

josh commented Oct 10, 2012

Still a cool idea, but I think its going to continue to live as a plugin for now.

@josh josh closed this as completed Oct 10, 2012
@mulderp
Copy link

mulderp commented Jun 1, 2013

Just wanted to check, if there are any new thoughts in the meanwhile about integrating CommonJS with Sprockets. I looked into https://github.com/maccman/stylo/blob/master/assets/javascripts/application.js , and it looks to nicely isolate components.

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