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

Refactor frontend pt. 2 #806

Closed
wants to merge 131 commits into from
Closed

Refactor frontend pt. 2 #806

wants to merge 131 commits into from

Conversation

tbekolay
Copy link
Member

Alright, so merging #673 also merged #805. I made an empty commit in this new PR so that shouldn't happen again.

I'm just going to continue on from where I left off in #805 so refer to that PR for the first part of the story. As before, feel free to click the unsubscribe button on the right to save your inbox.

@tbekolay
Copy link
Member Author

Actual commit finally! I now have npm+webpack mostly working in b7b8d7b. I did this in a minimally obtrusive way for now, which means that I kept all of the files in the same place and only made modifications, which you can see in the diff. Some of the lines in the diff can be removed now that I understand webpack better, which is part of the plan for tomorrow. Importantly, many lines are removed now since we no longer need what was in the nengo_gui/static/lib directory anymore -- that gets install with npm now.

With that commit, I can now start up nengo and everything works except for the built-in examples in the file tree (normal files work fine). I suspect this is due to some difference in how 1.0.1 and 2.1.4 work, which I'll investigate tomorrow.

One other issue that came up is that the JS bundle generated by webpack is ... bigger than I would have hoped -- about 2.4 MB. This is not minified at all, so we can compress that down significantly, but it makes me wonder how we should deal with it in the repository... In the ideal case, I think I would force Nengo GUI devs to install node.js so they can do npm install and then npm run build, then we can include the minified bundle when we do a release version, so that the end user isn't impacted at all. Something to discuss at some point anyway; for now, I'll commit them so people can test out the branch if they want to. If we decide not to include them in the repo, then it's easy to remove them all since all the outputs are in a single directory.

So, my next orders of business for tomorrow are to:

  • Get the file tree working
  • Clean up the new commit to have a smaller diff with some sick webpack trickz
  • Write a useful commit message for the new commit

Once those things are done, I will do some thinking about where best to move files. I think it'll be a lot more clear if we keep our JS and Python source files separate, instead of nested as they are now, aside from the generated webpack bundle which will be placed in the Python source tree. That should be easily done, just requires some agreement on the best place and the best names for directories and such.

@tbekolay tbekolay force-pushed the refactor-frontend branch 4 times, most recently from a56bf72 to 3c5c13b Compare July 14, 2016 20:09
@tbekolay
Copy link
Member Author

I managed to fix the issue with the new jqueryfiletree. We were using an <em> tag in the <li> associated with the examples, which jqueryfiletree was not expecting; the full fix is at 27090b8. I also discovered a bug in generating the SVG of the network, which now works properly. The big webpack + npm commit has an explanatory commit message (3c5c13b) and a much smaller diff (especially in terms of the number of files touched). Since I was able to get rid of the whole nengo_gui/static/lib folder, the diff stats are 300 lines added, 6,436 lines removed. Oh, and the unit tests pass, for whatever that's worth.

I was also able to somewhat mitigate the issue of the bundle being quite large... previously it was about 2.6 MB, but I now have a release task that generates the minified and optimized JS (see the commit message of 3c5c13b for more details), which is 1.2 MB. Still not great, especially because a minified JS will more likely create huge diffs, meaning that it might not actually be smaller in terms of the git history, which is what we care about. So I'm still not sure what is best for storing the bundled code.

In any case, my next order of business is to move the frontend files outside of the directory tree. Once they've been moved, then I'll start converting the JavaScript files to TypeScript. In getting the files to work with webpack, it became quite clear that with "modular JavaScript" (which is btw what using webpack leads up to), we can make our JS code look pretty similar to Python. More details below.

Proposed file moves

Here's the current state of the frontend files. The .js and .html files shown below; the .css are right alongside them. (note: generated with tree -P "*.js" -I "node_modules*" and manually adding in page.html)

├── build
│   └── sphinx
│       ├── doctrees
│       └── html
├── dist
├── docs
├── examples
├── nengo_gui
│   ├── components
│   │   └── tests
│   ├── examples
│   │   ├── basics
│   │   ├── recurrent
│   │   └── tutorial
│   ├── grandalf
│   ├── static
│   │   ├── ace.js
│   │   ├── components
│   │   │   ├── 2d_axes.js
│   │   │   ├── component.js
│   │   │   ├── htmlview.js
│   │   │   ├── image.js
│   │   │   ├── netgraph_conn.js
│   │   │   ├── netgraph_item.js
│   │   │   ├── netgraph.js
│   │   │   ├── pointer.js
│   │   │   ├── raster.js
│   │   │   ├── slidercontrol.js
│   │   │   ├── slider.js
│   │   │   ├── spa_similarity.js
│   │   │   ├── time_axes.js
│   │   │   ├── value.js
│   │   │   ├── xy_axes.js
│   │   │   └── xyvalue.js
│   │   ├── config.js
│   │   ├── datastore.js
│   │   ├── data_to_csv.js
│   │   ├── dist
│   │   │   └── nengo.js
│   │   ├── hotkeys.js
│   │   ├── jquery.js
│   │   ├── menu.js
│   │   ├── modal.js
│   │   ├── nengo.js
│   │   ├── side_menu.js
│   │   ├── sim_control.js
│   │   ├── test
│   │   │   └── datastore_test.js
│   │   ├── tooltips.js
│   │   ├── top_toolbar.js
│   │   └── viewport.js
│   ├── templates
│   │   └── page.html
│   └── tests
├── nengo_gui.egg-info
└── webpack.config.js

As you can see, some JS files are four levels deep, and they all exist in the nengo_gui directory, which also houses all the .py files, making that directory difficult to look through.

I'm thinking that we move the nengo_gui/static directory to the top level; the new name of the directory should reflect that the directory houses frontend files. It could be frontend, but I've seen other projects use the convention of www for frontend files, and I like it alright. I could be persuaded to change it if people have other ideas, but for the time being, I'll go with www and if better ideas come up, we can always change it.

In addition to moving nengo_gui/static, I think it makes sense to also move nengo_gui/template, as the only file in there is page.html, and I plan to remove the templating aspect of it eventually (subject of a future post). So I think nengo_gui/www makes sense for that.

Here's what it will look like when it's moved. I've done the move locally, but have to change some of the paths in webpack.config.js and package.json before making a commit with it.

├── build
│   └── sphinx
│       ├── doctrees
│       └── html
├── dist
├── docs
├── examples
├── nengo_gui
│   ├── components
│   │   └── tests
│   ├── examples
│   │   ├── basics
│   │   ├── recurrent
│   │   └── tutorial
│   ├── grandalf
│   ├── tests
│   └── www
│       ├── page.html
│       └── dist
│           └── nengo.js
├── nengo_gui.egg-info
├── webpack.config.js
└── www
    ├── ace.js
    ├── components
    │   ├── 2d_axes.js
    │   ├── component.js
    │   ├── htmlview.js
    │   ├── image.js
    │   ├── netgraph_conn.js
    │   ├── netgraph_item.js
    │   ├── netgraph.js
    │   ├── pointer.js
    │   ├── raster.js
    │   ├── slidercontrol.js
    │   ├── slider.js
    │   ├── spa_similarity.js
    │   ├── time_axes.js
    │   ├── value.js
    │   ├── xy_axes.js
    │   └── xyvalue.js
    ├── config.js
    ├── datastore.js
    ├── data_to_csv.js
    ├── hotkeys.js
    ├── jquery.js
    ├── menu.js
    ├── modal.js
    ├── nengo.js
    ├── side_menu.js
    ├── sim_control.js
    ├── test
    │   └── datastore_test.js
    ├── tooltips.js
    ├── top_toolbar.js
    └── viewport.js

Modular JavaScript

Right now, our code roughly follows the jQuery plugin model. We make a psuedo-namespace called Nengo, and our JS files read from and write to Nengo. Right now, the order in which things are added to that namespace is pretty critical; if one file expects the Nengo namespace to contain some variable, then we reorder the order in which the files are loaded, and if we're diligent, we'll add a comment saying that the order matters. However, there is no way to represent the dependencies between JS files except implicitly in the ordering of files.

With modular JavaScript, we get two primary benefits:

  1. We don't need to make our own psudo-namespace, as modules do not have access to other modules unless they explicitly ask for them with require (very much like importing in Python).
  2. The dependencies between files is explicit through the require calls, which means that we no longer have to have a big long ordered list of files to load (as exists in nengo.js right now).

So, making our files modular is my first priority. I'm going to do that with as few changes to the actual code as possible -- only making the modularity changes and nothing else. However, once that's done, there are two more steps in the frontend refactoring that I want to do: 1) adopt TypeScript to get access to ES6 features, like legit class declarations and default arguments, and 2) use custom events to keep different components in sync. Ideally, I can do those as two separate steps. More on those steps on later days.

@jgosmann
Copy link
Collaborator

www = "world wide web" seems like a misnomer to me as most nengo_gui instances will not be accessible as part of the WWW.

What will be the install location of www when the package is installed with python setup.py install (and what is the location of the nengo_gui folder for comparison)?

@tbekolay
Copy link
Member Author

www = "world wide web" seems like a misnomer to me as most nengo_gui instances will not be accessible as part of the WWW.

There's no reason they can't be, but I see your point. What would suggest as an alternative?

What will be the install location of www when the package is installed with python setup.py install

It wouldn't be installed, only the minified, optimized bundle would be installed as nengo_gui/www/dist/nengo.js.

@tbekolay
Copy link
Member Author

OK I did a little looking around at what other projects use ... unfortunately none of them stand out to me as being obviously better than other approaches. But here's a roundup of options:

  • Top-level frontend code with subdirectories
    • Top-level names: assets, client, modules, resources, static, src, www
    • Subdirectories: named by filetype (js, css, ts, less, sass) or more generic (scripts, styles)
  • Frontend code nested in Python folder
    • With all options above, and
    • static, templates subfolders

Jupyter notebook uses the static, templates approach that we use now, and Django suggests it for its projects. However, I feel that the static / template distinction is not really meaningful, especially in our case where we have one template file, which I plan to make a non-template file.

One other approach, which I haven't seen anywhere but sort of follows a Java-like directory structure, is a toplevel src directory with separate subdirectories for the frontend and backend stuff. So something like:

  • src/python, src/js, src/css
  • src/server, src/client
  • src/backend, src/scripts, src/styles

Anyway, since it's unclear the best name at the moment, I'll hold off on renaming things and move on to the modular JavaScript stuff next instead.

@jgosmann
Copy link
Collaborator

I think I like the top-level names client, frontend, and static best. I think for the frontend stuff I also like subdirectories by filetype because each filetype has some distinct purpose in this case (HTML for general page structure/mark up, CSS for styling, JS for frontend code) and seems quite common, but for the backend it seems to make less sense to me.

@tbekolay
Copy link
Member Author

tbekolay commented Jul 16, 2016

Separating concerns

One difficulty in editing the frontend parts of Nengo GUI is that some of the frontend objects are created in the backend. There are a few downsides to this, some of which are important to us and some of which are not. For one, it means that developers have to know how to read/write both frontend (JS) and backend (Python) code -- this doesn't matter so much to us now, but in the future we may attract more specialists.

However, an issue that does matter a fair bit for us is that it makes our frontend code difficult to test. I'll give an example here which will hopefully also explain what I mean by separating concerns: creating a value plot.

When you click on the "Value" entry in the right click menu, here's what currently happens:

  1. NetGraphItem.create_graph('Value') is called, which calls
  2. netGraph.notify with a JSON object that is sent to the server over a websocket.
  3. The server gets that JSON, does some things internally to set up the necessary data structures, then sends back a JSON object in NetGraph.update_client. The JSON object includes a string of JavaScript.
  4. NetGraph.on_message handles the websocket message. The JavaScript in the JSON object is evaled.

To some extent, these steps are all necessary, but following the paths through frontend and backend code to determine what is called when is difficult for mere mortal programmers, which in turn makes it quite difficult to test. Even worse, there are some parts of the frontend code, like setting the size of an object, that are done directly in the NetGraph.on_message function, making it even more difficult to test.

Aside: AJAX, REST and other silly acronyms

It's important to note here that this wasn't done for arbitrary "hey let's make this more difficult than it needs to be" reasons. These steps are necessary because of the asynchronous nature of JavaScript and client-server communication. JavaScript and the underlying technologies are designed for the internet, where you might be waiting for seconds to hear back from the server when you fire off a request for information. If JavaScript stopped everything it's doing while waiting for the server, it would take a ton of time to render even the simplest of modern websites. So, JavaScript fires off a bunch of requests to the server asynchronously, then uses callback functions to handle the responses that the server gives back whenever it's able. Most frequently, those requests are made with XMLHttpRequest objects, which contain the callback function that will be fired when the server responds to the request. These became so popular that people just could not stand saying XMLHttpRequest all the time, so they made up the term AJAX, which stands for Asynchronous JavaScript and XML, and basically means that the website communicates with the server through XMLHttpRequest objects. jQuery, for example, has a convenience wrapper around all this called jQuery.ajax. People also often talk about RESTful APIs when talking about AJAX, because it's a fast and scalable way to design a client-server architecture for a web application: your frontend makes AJAX requests to a REST (representational state transfer) backend that does CRUD (create, read, update, delete) operations on a database. That mouthful of jargon describes the majority of modern websites on the internet right now.

Nowadays, client and server code is straightforward to write if you use AJAX because everyone uses AJAX. On the client side, your callbacks are local to the AJAX request you're making, so you can structure your frontend code in a clear way. On the server side, AJAX requests are routed to the appropriate server function using URL components (e.g., Github knows what content to serve on this page because of the nengo/nengo_gui/pull/806 part of the URL of this page).

However, Nengo GUI doesn't use AJAX, nor should it; while AJAX's asynchronous nature makes it a natural fit for the internet, it still uses the HTTP protocol to transfer information, which imposes a lot of overhead when you're trying to push a huge amount data from the server to the client. Instead, we use a relative newcomer in web technologies to move data: websockets. However, being a new technology, the tooling around websockets is not as mature as it is for AJAX, nor is there a good set of conventions for writing client or server code using websockets. In the end, it means that we are essentially implementing the generic websocket routing and the Nengo GUI specific logic in the same place.

Separating concerns

OK, back to separating concerns. One set of concerns to try to keep separate are the generic websocket-y stuff and the core business logic. We have done a good job keeping those things separate in the backend server code, helped in part by #673 (which is to be expected given that we all have much more Python experience than JS experience). However, we need to do a better job of this in the frontend.

Another set of concerns is the coupling between frontend and backend. Who is in control ? I think because we are more familiar with Python, we have attempted to keep the backend in control of the application as much as possible. However, part of the reason that AJAX took off and became so popular is because it puts the frontend in control, which gives it much more flexibility in terms of providing a good user experience, which is in the end what we want to do. So, I think we need to switch control to the frontend for most aspects of the application.

There are a few concrete ways in which this manifests itself:

  1. Wherever possible, JavaScript should request information from the server when needed instead of being provided with information at arbitrary times.
  2. The server should never send JavaScript to be evaled in the client.

Examples of these two rules in action:

Right now, when the server loads up the page, it loads the template page.html and injects some JavaScript code in a function that gets evaluated on load. This function constructs the NetGraph, Ace editor, SimControl, and other objects. This is generated on the server, rather than in the client, because there are arguments to those constructors that aren't known by the client and must be provided by the server. Similarly, when you create a Value plot, as described above, the server sends a line of JavaScript over the websocket, which gets evaled.

After my refactoring, the control will be in JavaScript's hands. When page.html loads up, JavaScript will send a request to the server asking for the arguments to NetGraph, Ace editor, etc that it needs to construct those objects, then will construct those objects itself. Similarly when a Value plot is created, the JSON object that the server sends back will contain the arguments needed by JavaScript, but not the JavaScript itself.

The separation of concerns here gives us several benefits.

  1. The server would no longer have to know JavaScript, and consequently server developers would no longer have to know JavaScript. It would therefore be possible to develop a different frontend (e.g., an Android app or a desktop app) that uses the same backend server code.
  2. The frontend code would be easier to debug and test.

Specifically on the testing front, it would be possible to construct and test the frontend without communicating with the server at all; instead, we stub out what we would expect from the server and substitute mock data. While this is somewhat feasible with the current server, it requires intimate knowledge of the server, and therefore again requires that frontend devs also know how to read and write backend code.

RPC over websockets

I believe one way in which we can separate these concerns in the frontend is to use remote procedure calls, or RPCs, over websockets. The idea here is to essentially use websockets like AJAX calls, only without the overhead of making HTTP requests; it essentially does the generic routing operations that we would want to do in most cases.

I've found several implementations of this out there so far... the web application messaging protocol (WAMP) and associated tools (Autobahn and crossbar.io) are heavily developed with lots of features, but introduce relatively large dependencies. rpc-websocket gives us just the features we'd need, but is not as heavily developed.

My next order of business, after finishing up the modularization of the current JS codebase, is to investigate these RPC implementations and either choose one, or take the good parts from these and implement our own (rpc-websocket is only a few hundred lines of JS, so it's definitely doable).


As a brief update of the current situation, the modularization is mostly done; the only thing left is to update the server code to emit what is now the correct JavaScript (hence why I have been thinking about separating these concerns 😉).

@tcstewar
Copy link
Collaborator

Interesting. Thank you for the detailed description of your thought process! I definitely agree that a lot of the thinking behind the old design was to keep the server side in control as much as possible, because we're much more familiar with Python and its tools. And if modern JS tools make it cleaner to move the client to be in control, that sounds good to me. Thank you for digging into this!

@tbekolay tbekolay force-pushed the refactor-frontend branch from 1560a9c to 964a4ea Compare July 18, 2016 21:07
@tbekolay
Copy link
Member Author

tbekolay commented Jul 18, 2016

Modularizing JS is now complete! I've tried out pretty much everything I can think of in the GUI, and it works properly as of 964a4ea. I still have to write a decent commit message for that, but feel free to look at that commit now to see what I mean by modularizing JS... mostly it means not relying on global state, but because the frontend and backend are not yet fully decoupled, we still need to keep track of some global state, which is now encapsulated in the nengo object. By the end of the refactoring, there will be no nengo object, and hopefully little to no global state.

In addition, all of the new commits leading up to 964a4ea do fix some bugs I found and do various things to make frontend development easier. For example, running JSCS and generating the jsDoc is now trivial once npm is installed. Also, as of 13a25fa, all of the JS files that I've touched (which I think is all of them) pass the JSCS style checker.

I also have not yet gone through and changed the test files. I'm somewhat hesitant to do so, as the tests will definitely need to be changed again by the end of this PR -- and with any luck, a lot of the testing can move to pure JS, eliminating the need for Selenium in the tests we run on TravisCI (though we should keep Selenium tests around for running before releases, etc.) I'll look at them and see how much needs updating, and if it's a lot then I'll skip the tests for now.

The next steps are to write that commit message, then look into finishing the decoupling. However, one of my other goals in this refactoring is to switch over the JavaScript code to TypeScript. Because TypeScript compiles to JavaScript, I should be able to do this change without actually modifying the JavaScript files. I'm thinking that actually doing the decoupling is going to be easier with TypeScript, so that's going to be my next task -- in part to confirm whether or not switching over now will make the decoupling easier.

@tbekolay tbekolay mentioned this pull request Jul 23, 2016
@tbekolay tbekolay force-pushed the refactor-frontend branch 3 times, most recently from 2eb792d to 34dbc74 Compare August 4, 2016 20:25
@tbekolay
Copy link
Member Author

tbekolay commented Aug 4, 2016

OK, I'm finally back to work on this PR. I've now converted all the JavaScript files to TypeScript in 34dbc74. Like with the initial commit using npm and Webpack, I've attempted to do this with minimal changes so that the conversion to TypeScript is as straightforward as possible.

While the main reason for switching to TypeScript is to give us access to new language features (which 34dbc74 does not do), already the switchover has forced me to fix a few bugs that would have been difficult to spot otherwise. I'll talk about that and give a more thorough introduction to TypeScript tomorrow.

@tbekolay
Copy link
Member Author

tbekolay commented Aug 7, 2016

JavaScript as a low-level language

Before introducing TypeScript, it's worth giving a bit more context on the evolution of JavaScript, at least from my perspective.

JavaScript was famously created in 10 days 20 years ago as a way to add dynamic elements to websites that would execute in the browser (i.e., the client), rather than on the computer serving the website to the browser (i.e., the server). Since the language's "platform", so to speak, is the browser, it was designed with a very different set of concerns than most languages:

  1. Boilerplate should be kept to a minimum. JavaScript can live inside HTML tags (e.g., <a href="javascript:MyFunction();">link</a> and <a href="#" onclick="MyFunction();">link</a> are both valid and used all over the web) so it can't take multiples lines of code to do hello world.
  2. The physical machine is (mostly) irrelevant. Most languages need to have some concept of how data structures will be laid out in memory, how many bits each basic type will consume, and so on. But JavaScript's target platform is not a physical machine but the browser, so it can ignore those kinds of concerns and let the browser deal with it. The browser acts as an additional layer of abstraction between JavaScript code and the physical machine.
  3. Optimize for short scripts. The web and the computers running the web were completely different 20 years ago; no one anticipated that visiting a website would be pulling down dozens of megabytes of JavaScript code to be executed on the client -- and surprise! The server is also written is JavaScript with node.js. When JavaScript was designed, it was for short scripts interacting with ugly Geocities websites displayed on CRT monitors driven by Pentium-133 machines.

JavaScript today is not exactly what it looked like 20 years ago, but it's not very far off. Rather than making significant changes to the language itself (as has happened with Python), JavaScript's evolution has happened through 1) the community discovering the good parts of JavaScript, 2) high-quality open source packages using those good parts to make it easy to use JavaScript in novel ways (huge one-page websites, on the server with node.js, etc) and 3) browser developers optimizing the pants off of JavaScript to make it crazy fast.

A good example of how far JavaScript's come through these three avenues is asm.js and tools that use it. asm.js is a subset of JavaScript (i.e., some specific good parts) that runs at nearly-native speeds (2x slower than the equivalent C program), thanks to today's JavaScript engines. The emscripten project can compile LLVM bitcode (which can be generated for many languages including C and C++) to asm.js. Run emscripten on the MAME emulator, and now you can run hundreds of arcade games directly in your browser.

People rag on JavaScript a lot for being a bad language that was clearly designed in just 10 days with no forethought. And that's mostly true, but its unique set of concerns and high quality JavaScript engines (i.e., browsers) also give it some unique advantages. In order to best use JavaScript's unique advantages, many people have largely stopped thinking of JavaScript as a high-level language to be written directly, and instead think of it as a low-level language that should be generated by a high-level language that is easier for humans to write.

There have been lots of projects that compile (or, perhaps more accurately, "transpile") other languages (including Python) to JavaScript. However, these approaches are a bit short-sighted. First, there are situations where writing raw JavaScript is the right approach, and it's awkward to have a mix of languages in one project. Second, there are lots of high-quality JavaScript libraries out there, and we want to interact with them in an easy way. Third, there are actually innovations on the way for JavaScript the language, so when browsers are all supporting those new language features, it might be more enticing to switch back to JavaScript. In a way, what would be ideal is to write human-readable JavaScript and transpile to well-optimized JavaScript.

TypeScript

TypeScript gives us the benefit of writing in a human-readable high-level language and transpiling to low-level JavaScript, yet it avoids the downsides of other projects because TypeScript is a superset of JavaScript. This means that we essentially get to write human-readable JavaScript using new JavaScript features and transpile down to JavaScript that will work on current browsers.

Probably the most relevant new JavaScript feature for us is the introduction of the class keyword to write classes in a format much more familiar to us Pythonistas, rather than the prototype-based objects currently built into JavaScript (see this blog post for more details). There are clever uses for the prototype-based objects, but we're not using them, and the code will be much more readable once we make this switch, as most of the Nengo GUI code is organized like a traditional object oriented application.

One additional upcoming JavaScript feature is one that this PR has already discussed at length: modules. In addition to the benefits of modularization that I've already discussed, we no longer we have to worry about global variables that may or may not exist as the TypeScript compiler emits far more warning and error messages than webpack, and in general will disallow the use of global variable.

The TypeScript module syntax is slightly different than CommonJS's, but in my opinion it is better -- at the very least, it is much more like Python's import syntax.

Type definitions

What I've described thus far is essentially what the Babel project does -- enable future JavaScript features in current JavaScript. Going with Babel would be a perfectly fine choice. However, TypeScript also allows us to add type definitions to our code, which is the other primary benefit of switching to TypeScript. In short, type definitions allow us to explicitly mark variables as containing values of a certain type.

Adding type information seems out of place in the dynamically typed world of JavaScript and Python. Dynamic typing is great, and frees us from lots of unnecessary process in short scripts up to a thousand or so lines. However, when you start scaling up to larger applications (which I would argue the GUI has become), type information becomes very powerful, if not necessary. With larger applications, you're often interacting with parts of the code you did not write, or with third-party libraries. Often, you can glean a lot about how a function works if you know its name, the types of the arguments it accepts, and the type of the value it returns -- which is exactly what a TypeScript type definition is. There's a reason why documentation formats like jsDoc ask you to provide type information. And to preempt comments about how Python has managed to live thus far without it, Python docs often ask for type information, Nengo's Parameter system enforces types, and even Python itself can have optional type annotations (also check out the mypy project).

But I digress; the concrete benefits of adding type information for most developers is to enable code completion in editors (this is very difficult and/or slow in JavaScript) and to enable sanity checking in the TypeScript compiled (e.g., if you make a typo accessing a property of a class, the compiler will catch it and raise an error). Without that sanity checking or a robust test suite, you basically have to load the page and cross your fingers in JavaScript.

The specifics of how to include type information are detailed elsewhere (TypeScript quickstart is a good place to start quickly), but here are some really important concepts that aren't well spelled out in the docs:

  • When TypeScript builds your modules, it spits out a .js file, which the browser will execute, and a .d.ts file, which contains the type information.
  • If you're using a third party library, you need both the .js file and the .d.ts file to integrate it into TypeScript.

If you've done any C or C++ development, then a very apt analogy is the following:

  • Your .ts files are the source code.
  • The compiled .js file is an object file (the .o, .so, .dll, or .dylib).
  • The .d.ts type information is the header file in the #include.

As I mentioned, for your own TypeScript code, the compiler will spit out the .d.ts file (or you can import a .ts file directly), so there's no issue there. There is, however, an issue for third-party code, as most of the third party libraries that you would want to use are in JavaScript, not TypeScript. So where does the .d.ts come from?

Typings

The answer is: the community (usually). A whole ecosystem of projects (i.e., a bunch of packages and, yes, package managers) grew around providing type definitions for popular JavaScript libraries. The most well known is DefinitelyTyped, which has type definitions for nearly 2000 projects. A tool called tsd can be installed with npm and used to search and install type definitions from DefinitelyTyped.

However, as often happens with a centralized project that grows so large, a decentralized alternative was created, which is now the preferred option. It's called typings, and it can pull type definitions from several places including npm, Github, and yes, even DefinitelyTyped (which is why tsd is now officially deprecated).

For the time being, the set of steps involved in dealing with Nengo GUI increases by two due to the need to get .d.ts packages:

  1. After cloning, you have to do both npm install and npm run install-ts.
  2. After adding a new npm dependency through npm install --save-dev package, you have to install the appropriate type definitions and install them with typings install --save package-definitions.

Note that I've used typings as though it is available on the command line. This is only true if you globally install typings with npm install -g typings. If you haven't done this, you can access the project-local typings with ./node_modules/typings/dist/bin.js, which is executable.

While these extras steps are somewhat annoying, it's important to point out that they are -- hopefully -- temporary. In addition to the .d.ts file installed by typings, TypeScript will look at an npm package's "typings" key, which will point to a .d.ts file that TypeScript can then use. In other words, npm is now also a package manager to TypeScript projects, where a TypeScript project can be one written in TypeScript that builds and releases .js and .d.ts files, or it can be a JavaScript project that also maintains a manually written .d.ts file.

Writing .d.ts files

There is, however, one last case that you can run into. What if you want to use an unpopular or new third party JavaScript library that has no .d.ts file written for it? In that case, there's no way around it: you have to write a .d.ts file.

TypeScript has a decent document on how to write .d.ts files. That document, however, assumes that you're really into TypeScript and you want to make a JavaScript library fully supported in TypeScript. If you don't have time for that, then another option is to write a minimal .d.ts file (like the one in this StackOverflow answer). You can starts with the minimal stub and fill in the definitions for the functions that you use as you use them; that serves the dual role of making your code more likely to work (because TypeScript can do its sanity checking) and helping you learn about the library that you're using.


With all that context, I can now be explicit about what commit 34dbc74 does. The goal was the get all of the JavaScript files converted to TypeScript with minimal changes (as I did for the modularization).

While much of this was straightforward (rename .js to .ts, change require statements to import and change export style), I ran into more issues than I was expecting.

  1. TypeScript really didn't like our prototype-based objects, and I could not figure out how to get them working without converting them to future JavaScript classes, so that's what I did. However, I attempted to do this with as few lines changed as possible, so I kept the current indentation level of all of the functions even though they are now wrapped within the class declaration (I'll do all the reindenting in another commit).
  2. While type definitions for most of our dependencies were either easy to find, or not necessary to get the GUI to compile and run properly, the definitions for the Ace editor did not work because we import through brace. This required me to learn a bit of JS / TS / node stuff to convert the Ace definitions to something that works with brace. I'll be polishing that up and making it into a PR for the brace project next week. (If they don't merge it, then I'll make it an external definition.)

Those were some unforeseen downsides of the TypeScript conversion, but there were in fact several unforeseen benefits, as the TypeScript compiler found several subtle JS bugs. A few examples looking through the commit:

There were several instances of the main two issues (not using var, unintentionally using a global variable), so already I feel that moving to TypeScript was a good move. I should mention that the main downside of switching is that it requires a compilation step, but since we are using webpack for other reasons, we already have a compilation step, and integrating TypeScript with webpack was trivial enough that I didn't bother mentioning it to this point.

The next step will be start actually refactoring the code (!) to take advantage of both the new language features that we now have access to, and to add type information to ensure we're making the correct assumptions.

@Seanny123
Copy link
Collaborator

So, if one were to jump in one this occasionally to help you with this monumental, but supremely valuable task:

  1. Do you have minimal build instructions for this version of the project? I'm not asking for a step-by-step explanation. Just the basic commands to get started. I can search for the meaning of each of them myself.
  2. Do you have a checklist of what's left to be done? From what I could discern, this last commit was a major take-off point from the usual Nengo GUI business. So does that mean that refactoring is now the biggest priority? Do you have any thoughts in particular about what to refactor?

@tbekolay
Copy link
Member Author

It's not quite at the point where people can jump in, unfortunately; I have a couple thousand lines currently uncommitted because I'm ensuring that the GUI still works on each commit. Once it's in a state that works again I can think about places where people can pitch in, but this is the kind of task that is mostly about making sure all of the parts of the code work well in tandem, so it's hard to split off tasks that can be done in isolation.

@tbekolay
Copy link
Member Author

tbekolay commented Aug 22, 2016

This post will be a status update, rather than something educational! It's been a few weeks since my last post, so I wanted to check in, as this is still very high on my priority list. I figure that more people will want to start hacking on the GUI come the fall term, so I want to get this mostly wrapped up over the summer.

So what have I been doing? Well, I started by fixing the indentation in all the TypeScript files, as I mentioned at the end of my last post (see adf698c). With that "blank slate," I next looked into fixing up the TypeScript files in mechanical ways -- that is, getting the TypeScript files to pass as many automated checks as I could find without doing any real refactoring.

The first set of checks were those raised by the TypeScript compiler (tsc) itself. I had disabled six checks that tsc normally does in order to get the renamed files to build with webpack. Most of what was needed to get the compilation to work was to add all the type definitions through typings; I had done this for a few of our dependencies but not all of them. Tracking down the popular ones was not difficult, but there were three projects that did not have type definitions readily available.

The first project without definitions was brace, which is a version of Ace editor that works with webpack; since the Ace editor itself has type definitions, I was able to use those definitions and make some edits to get it working for the GUI; see brace#60 which will hopefully be merged and released soon. While the script to modify the Ace definitions is not very long in the end, figuring out how to write type definitions properly took way too long, mostly because the syntax and best practices have changed significantly in the past few years, which means that the tutorials and guides written online represent a particular snapshot in modern frontend development time and therefore may not actually work with the current versions of TypeScript etc. It was pretty triumphant when I finally figured out a set of definitions that compiled without errors!

The second and third projects were much easier to get working, in part because I had learned a fair bit from writing the brace definitions, and in part because both projects are jQuery plugins that have small APIs. Since they're jQuery plugins, they can't use the modular type definitions that are now considered best practice; instead, the definitions must be "global" (which is synonymous with "ambient", if you ever come across that term when looking up TypeScript or type definition information), which (as far as I can tell) have to be installed through typings. As such, there's little benefit to making a PR to those projects to add type definitions, so I instead made separate repos for those type definitions; see types-bootstrap-validator and types-jqueryfiletree. These can be used immediately and don't have to wait to be merged or released.

One annoying issue that I ran into while writing these definitions -- which I still haven't figured out -- is that I still get compile errors in Emacs even though the project builds properly through webpack. What I've figured out so far is that the Emacs plugin for TypeScript uses TypeScript's tsserver rather than the usual tsc compiler. The main difference between the two seems to be how they locate the tsconfig.json file that points to all the type information... however, tsserver seems to finding the right tsconfig.json file, yet still gives different output. A few things that I'm going to look into next to fix this issue are to 1) see what versions of TypeScript various tools are using and see if they can be made to use the same version (in the JS world, it seems as though all projects grab their own separate versions of each dependency in order to have reliable builds, at the cost of disk space) and 2) see if there is some configuration that I can add to tsconfig.json to get them working the same. It's possible that the ts-loader that we use in webpack uses different defaults for some configuration options than tsserver does.

Having fixed those issues, I then looked into the current state of linting in TypeScript. Fortunately, this proved much easier to figure out than the type definitions. The de facto linter is TSLint, which integrates easily with Emacs, and can be run on all TypeScript files through the command line. I've added a rule to our package.json to run TSLint with npm run style (which also means that it'll be trivial to run in TravisCI). Some of the TSLint rules are a little kooky coming from a Python background (camel case?!) so it might be worth thinking a bit more about what rules we actually want to use moving forward. TSLint is quite easy to configure, including the ability to create custom rules, which may be worth doing when we start developing the plugin system more. In any case, before finalizing the current style changes, I at least want to look at the existing additional rules plugins out there (specifically tslint-eslint-rules, tslint-microsoft-contrib, and vrsource-tslint-rules).

With the linter set up, I went through all the source and fixed up the style issues. There was a lot (see the commit, b13b815), so doing it all took a few days in itself... but what ended up being the most time (and energy) consuming was that once it was all lint-free, I built it, started it up, and... the GUI didn't work.

At first, I couldn't make any components -- the slider would either appear and be as big as the whole screen then disappear, or it wouldn't appear at all. This ended up being due to a wrong assumption on my part. In the component superclass (nengo_gui/static/components/component.js), there are width, height, w, and h attributes. I assumed that this was a mistake and changed all usages to width and height, but in actuality the w and h variables are not the same as width and height. Perhaps they should be (I will figure that out when I refactor these classes for real), but for the time being, they are not, and so after a few days of fruitless debugging, I just restored the original logic, ensuring that w/h and width/height were used in the same places they were used before.

Once I got that working, I could make components, but... no data was being plotted when I pressed play. In this case, I managed to track it down to the datastore classes giving me a ton of NaNs instead of numbers. Fortunately, there are tests for the datastore classes! I got those tests running in the current version of the code, and it didn't take too long to figure out my mistake in de-linting those classes.

After this, basic functionality in the GUI was restored, so I felt okay making a commit. I know for a fact that some parts of the GUI are not working correctly (pressing space in the editor plays/pauses the simulation, for example), but the datastore tests reminded me of how ridiculously useful having a test suite is when making so many big changes. So, rather than pull my hair out manually testing the GUI and debugging issues that I find, I'm going to take a step back and start writing tests for the rest of the frontend code. This will likely be necessary for the actual refactoring anyhow.

So, my next post will be another educational one, and it will be about testing in modern frontend development. Like all of the other headaches I've talked about above, getting tests to run in modern frontend dev is far from trivial... but once you somehow happen upon the right combination of tools and set them up properly, everything works very nicely. More about that soon-ish!

@jgosmann
Copy link
Collaborator

This ended up being due to a wrong assumption on my part.

I think most debugging is about identifying wrong assumptions. 💭

w and h variables are not the same as width and height

I'm pretty sure that all of that size calculation stuff needs some refactoring. There is a bunch of coordinate systems (relative to the parent object, on screen pixels, etc) and it is often not clear what is expressed in what coordinate system. There are three functions to get the size of a netgraph item (get_scaled_width/height, get_nested_width/height, get_displayed_size) with no documentation and I can't tell what exactly each of these is returning just from the name.

So, rather than pull my hair out manually testing the GUI and debugging issues that I find, I'm going to take a step back and start writing tests for the rest of the frontend code.

Thank you! ❤️

@tbekolay
Copy link
Member Author

I'm pretty sure that all of that size calculation stuff needs some refactoring.

Yes, agreed, Terry's said the same in the past. I'll have to dig in more to think about how best to do this; my only experience with that kind of stuff is from Matplotlib's transformations system, but it's unclear at the moment whether something like that would be appropriate for the GUI.

Using the <em> tag caused an issue with newer versions of
jqueryfiletree because they look at the direct parent of the
element that got a click event; since that element was the <em>
tag, the parent was <a>, but jqueryfiletree expected that to be
the <li> element, and therefore could not determine if the <li>
corresponded to a file or directory.

Fixed this by removing the <em> tag from the built-in examples
directory entry in the file tree, and instead use a CSS rule to
make that link italicized (but not any descendants).
@tbekolay tbekolay mentioned this pull request Sep 26, 2017
@tbekolay
Copy link
Member Author

tbekolay commented Aug 7, 2018

As was discussed in a recent meeting, I've split this refactoring (which is perhaps more accurately a rewrite) into a separate repository. Please go there if you're interested in working on it.

Not 100% sure how we'll deal with merging or switching over at some point, but splitting it off into a separate repo doesn't limit our options, I believe.

@tbekolay tbekolay closed this Aug 7, 2018
@tbekolay tbekolay deleted the refactor-frontend branch August 7, 2018 15:03
@tbekolay tbekolay restored the refactor-frontend branch August 7, 2018 15:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

4 participants