jsx offline transform exits with error code 1 on any change (Ubuntu 12.10)

[akrieger]

Basically what the title says. After install react-tools, using jsx -w will correctly detect existing files (but not transform them), and upon any write to any watched file jsx will exit with error code 1. Commoner tests all pass.

Exact steps I followed, in Ubuntu 12.10:
0) Install node.js via manual instructions at https://github.com/joyent/node/wiki/Installing-Node.js-via-package-manager for Ubuntu/Mint

1. Install react-tools with 'sudo npm install -g react-tools'
2. Create file helloworld.js in , as described by the Getting Started tutorial.
3. In a new terminal, jsx -w
4. Edit helloworld.js and add a comment.
5. Notice that jsx exited, echo $? prints 1.

akrieger@vb:/www$ ls js/
helloworld.js
akrieger@vb:/www$ ls html/js/
akrieger@vb:/www$ jsx -w js/ html/js/
["helloworld"]
<edit helloworld.js, save edit>
akrieger@vb:/www$ echo $?
1
akrieger@vb:/www$ ls html/js/
akrieger@vb:/www$

[benjamn]

Hypothesis: The only thing that keeps commoner from exiting in --watch mode right now is the { persistent: true } option passed to fs.watch. That means the process will stay alive while there are any files left to watch. But some text editors save files by deleting and then re-writing them, which causes fs.watch to un-watch the file (because the original inode that inotify cared about got discarded).

When there's only one file, unwatching it causes Node to think no files need to be watched, so the process exits.

cc @jeffreylin

[benjamn]

Opened a new Commoner issue: https://github.com/benjamn/commoner/issues/18

Even though @akrieger said the Commoner tests pass for him, that's only because --watch mode test coverage is… lacking. 😦

[akrieger]

That didn't seem to work for me, unless the file is supposed to be a valid JSX file with something to transform.

[akrieger]

It also doesn't solve the issue that jsx doesn't seem to be doing any static transformation anyway, even in the example getting started tutorial.

[benjamn]

That's another regrettable but known issue: I suspect you're missing the /** @jsx React.DOM */ comment at the top of the file (which won't be necessary soon, but currently is still important).

cc @zpao

[akrieger]

I copy pasted the sample code exactly:
/** @jsx React.DOM */
React.renderComponent(
blah blah
);

[benjamn]

Just to try something else: bin/jsx can take a single file name on the command line: bin/jsx app/helloworld.js. Does that also not show any transformations? Or did you try that already?

[akrieger]

bin/jsx js/helloworld.js does the right thing.

[akrieger]

Furthermore, if I have a file named helloworld.js in the destination directory, it is deleted when I try to run jsx on the source and dest directory, in either watch or go mode.

[benjamn]

The deletion is by design, at least. The files that end up in the output directory are actually hard-linked with master versions that are kept (by default) in ~/.commoner/module-cache/*.js. Each build unlinks and then relinks the files in the output directory, so if the relink fails it will look like the file was just deleted.

Not sure why the relinking might be failing.

In case ~/.commoner is on a different device from node_modules, you could try relocating the cache:

bin/jsx --cache-dir module-cache -w js/ html/js/

[akrieger]

Yeah, they are on different devices. Moving the cache dir worked. Thanks!

[benjamn]

Accidental close, sorry. Still need to mitigate that cache dir problem.

[petehunt]

How important is the cache behavior? Could we disable it? I'd trade perf for ease of use with this tool

[zpao]

👍 to what @petehunt said. I'd also take defaulting the cachedir to the src/current dir (I think this is how sass does it's cache)

[benjamn]

@petehunt pretty important; the incremental speedup is significant for a codebase the size of react-tools/src/

It's a one-line change to switch the default location, and it won't break anything for anyone. Kicking myself now because I actually considered whether ~/.commoner was likely ever to be a different device, and clearly I guessed wrong :(

[petehunt]

I wonder if the requirements that we need for the react codebase are the same as those for end users. I think most people just want a simple syntax transform and watcher ala coffee script.

[petehunt]

Also +1 on just moving the cache dir :)

[benjamn]

@petehunt true, but I don't think we can pretend that bin/jsx is a fully-featured source transform tool if it doesn't do any caching (coffeescript and sass clearly suggest that caching is worthwhile for enough people)

The original location of the cache directory was outputDir/.module-cache, but I moved it when I added support for printing the transformed code to STDIN, in which case there's no outputDir. Definitely okay with skipping the cache logic in that case.

[jeffreylin]

So I've been looking through various implementations of Dir watchers (gaze, chokidar) in NodeJS and they're all pretty clowny - a pretty easy test case that breaks them is making a new directory and adding files to that directory.

So since someone on the internet was wrong, I wrote another FS watcher last night that I think is way more stable:
https://gist.github.com/jeffreylin/5df1c95b57982720d444

I'll try to merge this new file watcher with the transformer I made a couple days ago (https://github.com/jeffreylin/transformer_fun) and see what happens just as proof of concept.

@akrieger - I'll probably have something for you to play w/ in an hour or two =]

[petehunt]

I had pretty good results with a makefile and running make every 2s :P

[benjamn]

@petehunt now you're trolling me :P

[akrieger]

@jeffreylin, I can't wait.

[petehunt]

Not trolling; it's very reliable and easy to reason about. Is comparing file mtimes to determine if they need to be rebuilt bad relative to a module cache?

[jeffreylin]

@akrieger give https://github.com/jeffreylin/jsx_transformer_fun a shot - still a work-in-progress, but lmk how it goes

[benjamn]

Dear @petehunt,

Your question deserves a careful response. I know this looks long-winded, but if you read it through you'll know everything that I was thinking.

The primary virtue of the module cache is that the cache filenames are hashes of all the relevant input variables: source, module name, build steps, configuration properties, Commoner version, and more. Change any one of those input variables and you get a different hash. If you find a file called <hash>.js in the module cache, you can be completely certain that the cached file contains the right contents. I believe that's a useful guarantee.

A secondary benefit is that old versions of a file don't go away just because the file was recently rebuilt. If you revert a file to a version that has ever been built before, you don't have to rebuild it again. In order to make the same promise under the mtime strategy, you'd have to find some way of storing all those distinct versions, and I'm confident the method you would come up with would closely resemble my hashing strategy. It's what every sensible version control system does.

The goal of both strategies is to determine whether you can trust the contents of a file that was built some time in the past. This decision is completely reliable under a hashing strategy, and if it ever fails then there must be some additional piece of information that you neglected to incorporate into the hash. None of the bugs that I've fixed in Commoner had anything to do with module cache inconsistency. When's the last time you had to wipe your module cache? And no, grunt clean doesn't qualify, because I made sure it left the cache untouched.

With that background, let me answer your orginal question: "Is comparing file mtimes to determine if they need to be rebuilt bad relative to a module cache?" Yes, because the only input variables considered by the mtime strategy are (name of source file, mtime of source file), which allows you to avoid rebuilding the file if

1. the corresponding output file has a later mtime, and
2. you know no other input variables have changed in the meantime, such as build steps, configuration properties, new files added/removed whose presence/absence affects the build, independent build processes racing to update files in the output directory, etc.

Requirement 1 is too strong because you don't necessarily have to rebuild source files that appear newer than their output files (see my second paragraph above).

Requirement 2 is too strong because you don't have to "know" any of those things if they can be accounted for automatically, as they are under the hashing strategy.

Is the mtime strategy good enough? Maybe it is for you, because you know how to spot signs of inconsistency and you have trained yourself to predict when a make clean is finally necessary. I don't think we can expect the same of new users of React. They're much more likely to attribute subtle problems to the library itself instead of guessing that the build tool messed up.

Sometimes we reinvent the wheel just so that we know exactly how the wheel we're using works, quirks and all. I'm guilty of this myself, but in my defense I really am trying to get us to a point where we don't have to make any apologies at all for the way our tools work. I know there are still bugs in Commoner, but I'm committed to squashing all of them.

[petehunt]

I'm not hating on commoner. It's an excellent CommonJS build system and is totally the right choice to build react and any jsx project that uses CommonJS modules with transforms.

The problem is that users think it is a simple syntax transform (like coffee?) when in reality it doesn't behave that way. Specifically it assumes that everything you're desugaring is a CommonJS module and it assumes specifics of the implementations path resolution methods, which in requirejs, browserify and haste are all different, by relativizing.

If you aren't using CommonJS and define your own unrelated require() function the arguments will get rewritten, weird missing module warnings will fire and fake modules will be created. It's actually really freaking awesome to have this tooling for CommonJS projects but really confusing if you're not in a CommonJS project or using a different module resolver. Some of the annoyances we've faced with r.js are because it tries to be too clever in this area (we have eval('req'+'uire')(...) somewhere in the insta codebase because of this).

As a user since I know jsx is willing to rewrite my code I find it hard to predict exactly how it will transform.

What if instead jsx had a --relativize option that we disabled by default and it just treated everything like a file and transformed it, either with a cache or based on mtime? We'd continue to use the option on react itself and perhaps even advertise commoner+a browser packager (brigade?) as our recommended module system since they'll likely want debranching and other goodies even though they don't know it yet :)

[benjamn]

The original issue still needs fixing, by the way. Still working on that.

[ankittanna]

One of the reasons I was not able to get the jsx transformed files was I didnt close the terminal window first time after installation. I closed the terminal window and re-opened it again and then I was able to transform any JSX file located anywhere on my disk to any folder. :) possible solution.