Coder Social home page Coder Social logo

Comments (28)

benjamn avatar benjamn commented on December 14, 2024

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

from react.

benjamn avatar benjamn commented on December 14, 2024

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. 😦

from react.

akrieger avatar akrieger commented on December 14, 2024

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

from react.

akrieger avatar akrieger commented on December 14, 2024

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.

from react.

benjamn avatar benjamn commented on December 14, 2024

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

from react.

akrieger avatar akrieger commented on December 14, 2024

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

from react.

benjamn avatar benjamn commented on December 14, 2024

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?

from react.

akrieger avatar akrieger commented on December 14, 2024

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

from react.

akrieger avatar akrieger commented on December 14, 2024

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.

from react.

benjamn avatar benjamn commented on December 14, 2024

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/

from react.

akrieger avatar akrieger commented on December 14, 2024

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

from react.

benjamn avatar benjamn commented on December 14, 2024

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

from react.

petehunt avatar petehunt commented on December 14, 2024

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

from react.

zpao avatar zpao commented on December 14, 2024

πŸ‘ 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)

from react.

benjamn avatar benjamn commented on December 14, 2024

@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 :(

from react.

petehunt avatar petehunt commented on December 14, 2024

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.

from react.

petehunt avatar petehunt commented on December 14, 2024

Also +1 on just moving the cache dir :)

from react.

benjamn avatar benjamn commented on December 14, 2024

@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.

from react.

jeffreylin avatar jeffreylin commented on December 14, 2024

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 =]

from react.

petehunt avatar petehunt commented on December 14, 2024

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

from react.

benjamn avatar benjamn commented on December 14, 2024

@petehunt now you're trolling me :P

from react.

akrieger avatar akrieger commented on December 14, 2024

@jeffreylin, I can't wait.

from react.

petehunt avatar petehunt commented on December 14, 2024

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?

from react.

jeffreylin avatar jeffreylin commented on December 14, 2024

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

from react.

benjamn avatar benjamn commented on December 14, 2024

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.

from react.

petehunt avatar petehunt commented on December 14, 2024

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 :)

from react.

benjamn avatar benjamn commented on December 14, 2024

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

from react.

ankittanna avatar ankittanna commented on December 14, 2024

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.

from react.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    πŸ–– Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. πŸ“ŠπŸ“ˆπŸŽ‰

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❀️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.