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

Beefy 2 -- The Beefening #40

Merged
merged 49 commits into from
Jun 13, 2014
Merged

Beefy 2 -- The Beefening #40

merged 49 commits into from
Jun 13, 2014

Conversation

chrisdickinson
Copy link
Owner

This is a really big refactor of beefy. The goal is to put beefy on a better footing to achieve the following goals:

  1. Easier to integrate beefy into an existing project.
  2. Easier to add new features to beefy without causing regressions.
  3. Easier to test beefy.
  4. Better windows support.

Addresses:

  1. Chokidar was removed in favor of watchr. It was having issues that were tripping assertions in libuv (!!!), and it necessitated a download of a node distribution during install. Not worth it at the moment. refs: switched to chakidar for file watching #38
  2. Add default name extension. Finally. Sorry for taking so long, @maxogden. refs: support default extensions #5.
  3. Should kill the max listeners warning. refs: Max listeners warning is annoying. #22.
  4. You can specify a different default generated index.html with -i path/to/index or --index path/to/index.

Immediate Road map

for this merge, set in stone!

  1. Better docs / introduce a HACKING.md with notes on what does what, where.
  2. Grow a favicon because fun. refs: favicon.ico pulling from different directory #17
  3. Integrate watchify mode. Get windows support working. These are related because they may cause us to have to move away from the child_process.spawn style of bundling. refs: beefy-two and watchify #39, Windows Compatibility? #14

Future Road map

post-this merge, may change!

  1. Add --test handler, that exposes /-/report so beefy can be used for semi-automated testing.
  2. Add support for other handlers (sass, hbs, etc).
  3. Potentially add support for a ~/.beefyrc and "beefy" package.json directives (and, potentially, a beefy install to install beefy in your project).

@max-mapper
Copy link

AWESOME!!!!

what is the alternative to the child_process.spawn method? the browserify programmatic API?

feedback for the post-this merge section:

#1: might be nice to have something like this: mantoni/consolify#3
#2: IMO these shouldnt be in core, would be nice if there was a way for 3rd party modules to do it
#3: (beefyrc, beefy install), what is the problem being addressed exactly?

@chrisdickinson
Copy link
Owner Author

Whew. So, watchify is now supported, which handily makes windows work as well.

The resolution order is as follows:

  1. Did the user supply a --bundler (or --browserify)? If so, we're in legacy (non-windows) mode, and we'll be using child_process.spawn.
  2. Look for a local watchify.
  3. Look for a local browserify.
  4. Look for a globally available watchify.
  5. Look for a globally available browserify.
  6. At the moment, there's an error. But this would be a great place to put a more prescriptive error -- "hey! npm install watchify!"

We're using the programmatic API by way of using browserify to parse our CLI args. The new bundle.js should be flexible enough to accept any module that can produce javascript given a file path.

@mikolalysenko
Copy link

When is this going to land? It looks really great!

@chrisdickinson
Copy link
Owner Author

@mikolalysenko Hopefully soon. There's actually two blocking issues, and one that I'd like to get some feedback from yourself, @hughsk, @substack, and @maxogden on:

Beefy currently uses whatever local browserify or watchify is installed, falling back to the system browserify/watchify. One of the main goals of beefy2 was to get this behavior working on windows, where child_process.spawn doesn't work very well at all for calling other node scripts, especially not as beefy was using it. Beefy moved to requiring watchify/browserify after locating it (with the usual "find local modules, then find globally installed modules" pattern), and using the command parsing from browserify directly -- which is to say, it looks for watchify first, then once watchify is found, it finds watchify's browserify, and then uses that browserify's bin/args.js to parse the arguments. This feels really fragile to me, and I'd like to find a better way forward -- maybe some intermediary package that parses out browserify CLI arguments? Or is this fine for the time being? The other option is to peg beefy to a specific version of watchify, which ensures that beefy will always work, but that it'll no longer use the locally installed bundler by default, which decreases its utility as a dev tool (IMO).

The other blocking issue is that once I switched laptops, the tests started hanging due to chokidar/fsevents not releasing the event loop properly.

@chrisdickinson chrisdickinson merged commit 0fc7f97 into master Jun 13, 2014
@mattdesl
Copy link

👍 awesome stuff. does this mean watchify and all these other fixes are now in current beefy on npm?

@chrisdickinson
Copy link
Owner Author

@mattdesl Thanks! Current beefy on npm will use watchify, if it detects it's available:

  1. it'll check your local repo first for watchify, then browserify.
  2. if neither are found, it'll use global watchify, then global browserify.

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