mortalapeman / LT-TernJS

A TernJS plugin for Light Table
Other
32 stars 4 forks source link

Plugin loads more JS files than necessary and does so synchronously #5

Closed mortalapeman closed 10 years ago

mortalapeman commented 10 years ago

When first started, the plugin reads all .js files found in one's workspace synchronously. Currently all directories are read in, including the node_modules directory if it exists. Tern's node plugin appears to be able to load modules just like node, so we can probably exclude the node_modules directory completely when doing our .js file crawl.

It would also be preferable to move to an async file search, probably utilizing LT's background macro. That way we don't lock up the UI when initializing the Tern server.

EDIT: This issue originated from a problem described here but has since been repurposed. The root cause of the original issue has proven to be elusive.

joshuafcole commented 10 years ago

I'm able to reproduce the stack bust with the following dependencies:

{ "dependencies": { "async": "~0.2.9", "trans": "0.0.16", "qs": "~0.6.5", "newrelic": "1.3.0", "underscore": "~1.5.2", "request": "~2.27.0", "deap": "~0.2.0", "deepval": "0.0.5", "moment": "~2.5.1", "xml2json": "~0.4.0", "marked": "~0.3.1" }, "devDependencies": { "qs": "~0.6.5", "mocha-as-promised": "~1.4.1", "chai-as-promised": "~4.1.0", "wd": "~0.2.2", "yiewd": "~0.2.0", "monocle-js": "0.2.0", "colors": "0.5.0" } }

I'll see if I can narrow that down to a particular subset now so we can glean some information from whichever dep(s) might be causing the problem, or if it's just the number/size of deps causing the hang.

joshuafcole commented 10 years ago

Nix that last, sorry. I'd left the old workspace open instead of creating a new workspace for testing. It doesn't seem to be the dependencies so much as the structure of the app itself.

Unfortunately, it's written for a company framework that hasn't been open sourced yet that does some admittedly weird file system transforms to build client side code out of a unified package. I imagine something about that transform, or about the AMD style (require js based) client side code is causing issues here. I don't really have a way to separate client and server code to test that since code is grouped by functionality instead of target.

If I can figure out how to get you more information without being able to provide code samples I definitely will, and if time permits I'll poke around with it some more to see if I can figure out the root cause. Otherwise, I think you're probably safe to write this one off for now and focus on other improvements.

Sorry for the trouble, Josh

mortalapeman commented 10 years ago

No trouble at all. I'm quite curious to see an example of this issue if you can isolate it. In the mean time, I'll repurpose this issue for an improvement that your discovery has pointed out.

ackalker commented 10 years ago

Perhaps a dumb question, but why not use a .tern-project-like file? Many people working with Tern from before Light Table will already know how to use it. See: http://ternjs.net/doc/manual.html#configuration , in particular the loadEagerly and dontLoad options.

mortalapeman commented 10 years ago

@ackalker Not a dumb question at all. When I started working on this little project, I had a couple of reasons for building my own node server.

  1. I had no idea what this project was going to look like so I went with the simplest thing I was comfortable with to reduce the apparent complexity of the problem.
  2. I was unsure whether or not I'd need to modify the HTTP server file that comes with tern to serve some purpose I had in mind.

Now that I have a much better grasp on this whole project, I need to look back into using the HTTP server. Thanks for reminding me ;)

mortalapeman commented 10 years ago

@ackalker After looking back into the HTTP server provided by tern, I now remember why I chose not to use it. The HTTP server looks for a .tern-project file by walking up the file tree from its current working directory and examining the files in each directory. There is no way to configure the HTTP server to just use a project file provided by the first HTTP request.

To make use of the built-in .tern-project file functionality, we are required to assume that whatever directory includes the .tern-project file is your root project directory. The concept of a Light Table workspace allows for zero to many directories to be included. If we must choose at least one directory as root, other directories containing JavaScript files might be left out. Using the HTTP server also makes configuration of a workspace with only files problematic. Thus, the HTTP server provided by tern is rather limited for our purposes

ackalker commented 10 years ago

Well, as far as I understand it, Tern will always need some information about the structure of the project in order to infer dependencies across files. Already this can be a bewildering mix of configurations (depending on which Tern plugins are used): node_modules hierarchies, AngularJS/ComponentJS/RequireJS mappings, etc. In the future, this will only get worse, as more and more frameworks and build systems will be added (Architect, Engineer, hr.js, ...) Unless the intention is to gather this dependency information entirely within Light Table, I don't think there's an easy way out; Tern will need to be told where to look and what to include/exclude.

Why not simulate the existence of (multiple) .tern-project files, perhaps even with a VFS? Yes, that may even mean starting multiple Tern servers, but that's a risk one already runs by having a workspace with a huge project involving both front-end and back-end code, all using multiple frameworks, anyway.

Personally, I think that the benefits of taking the HTTP server from Tern (no need for it to actually serve anything itself, we just need its configuration and setup code, then use our own communication and management) and adapting it to the needs of Light Table far outweigh the probably monstrous effort it would take to gather and store all this dependency information in Light Table itself...

ackalker commented 10 years ago

Ah, I see now that you did take the code from the HTTP server, I assumed that you rolled your own...

mortalapeman commented 10 years ago

@ackalker I think you underestimate Tern ;). From my minimal testing, Tern is able lazily resolve local CommonJS dependencies just fine without the need for a .tern-project file or even inclusion of the node_modules directory in the workspace. I suspect AMD style dependency loading works in a similar way since both rely on the location of the current file (as I understand the specs).

I'm not opposed to providing a custom way to read in .tern-project files if they exist, we just don't want to limit our options by using the built in server. Also, I did roll my own, I just happened to use similar function names and the same timeout of 5 minutes.

mortalapeman commented 10 years ago

The commit c5d9a6f provides support for asynchronously walking the workspace to determine what files to send to the tern server. We now ignore .git, .svn and node_modules which greatly reduces the number of .js files eagerly loaded.