Make build build again? (plus some)

[evs-chris]

I don't know if it's something I did wrong locally, but build ceased working for me for a few reasons, the first of which was a short bout of dependency hell (staleness; not related to the first commit here that adds the source-map-support dep).

The main problem seemed to stem from the source node not kicking off when the build started. Once that clears, I found that flattening sourcemaps kills the build for me because I have some external dependencies that are getting concated and uglified that don't have the referenced map sitting next to them. So, for now at least, I figured it was better to toss the error to the console and roll on with the build.

My build is an extra-funky 49 stages, since there are client, server, and shared sources of various types that get copied about and transformed. That makes figuring out which moveTo or merge path goes where a little tricky, so I added the option to supply an id to built-ins to make them a little easier to track down. I also applied the same logic to the main gobble export to allow multiple nodes to be supplied without passing them explicitly as an array because I keep face-palming when I can't figure out why a stage that I just added some more source to (the second node for the stage, of course) doesn't seem to work.

So, this gets build working for me again. There is a related PR incoming for the cli. If the fixes here are correct but you don't like API additions, please feel free to cherry-pick 'em. I added them for debugging and thought they might be nice enough to let them hang around.

[Rich-Harris]

Removing node.start() from build.js was pretty daft on my part. Sorry! I've opened #50 for the proper fix, and will merge that commit in the meantime.

I found that flattening sourcemaps kills the build for me because I have some external dependencies that are getting concated and uglified that don't have the referenced map sitting next to them

Oh, another sorry. Yeah, it probably doesn't make sense to fail the build in that scenario, given how finicky sourcemaps can be. Would this qualify as a bug in sorcery? (Or maybe it should have a 'don't fail if the map doesn't exist for some reason' mode?)

My build is an extra-funky 49 stages

Eeesh. Well that's one way to find weak spots!

I'm in two minds about the API change - have opened a separate PR that incorporates the other fixes (#51), will do a separate branch for the multiple nodes thing so they can be discussed separately

[evs-chris]

Would this qualify as a bug in sorcery?

My opinion is not really. I would go with the 'sourcemaps are critical' mode in gobble. It might be nice to have a way to say 'I don't care about sourcemaps on this node ever, so strip 'em out please' for those cases like external dependencies.

Eeesh. Well that's one way to find weak spots!

Yeah, I may be abusing it a bit. But, I do have a nice friendly source tree for the other guys on the project. Everything is ES6-ified, there is a custom Ractive template builder/loader stage, external dependency fetching, and node-server to run the server side. Gobble makes it quite nice. The whole gobblefile is only 92 lines with extra sections for production vs development. I shudder to think what a gruntfile would look like.

I'm in two minds about the API change

Me too. I've just gone from gobble('some/src') to gobble('some/src', 'ohyeah/this/too') and spent 45 minutes trying to figure out why the end result was borked more times than I care to admit. I appreciate the simplicity of two fixed arguments.

The built-in node id override I feel are probably less useful. I just happen to have 7 grabs, 15 moveTos, and 9 merges and should probably use a simpler project to bug hunt next time :)

[Rich-Harris]

Actually there's no need for a separate branch, now that master has been merged into this.

The bit I'm slightly wary about is gobble(n1, n2) - there have been lots of times when I've tried to make APIs more accommodating and sworn 'never again' when they turn out to be a maintenance headache and source of weird bugs. What if gobble threw an error if you tried to use it like that, and suggested doing gobble([n1, n2]) instead?

I like the idea of being able to pass an ID through to built-ins, so passing an options object makes sense. I was wondering if we should just ditch the ability to pass multiple strings:

// this...
node.grab( 'foo/bar/baz', { id: 'test' });

// ...or this...
node.grab( path.join( 'foo', 'bar', 'baz' ), { id: 'test' });

// ...instead of this:
node.grab( 'foo', 'bar', 'baz', { id: 'test' });

It would make the internals a bit simpler, and I suspect we'd thank ourselves in the long run. Moreover I don't think I've ever actually needed to pass multiple strings (but maybe that's just me?)

[evs-chris]

Now that I think about it, I always pass slashy string paths because they work everywhere. I wouldn't miss path joining at all.

An error would also probably be better than trying to reverse engineer optional intent.

[evs-chris]

I think the checks and single path thing definitely the best way forward, so I'll close this.