Major Project Restructure and efficiency related changes#2083
Closed
menasheh wants to merge 8 commits intojmoenig:masterfrom
Closed
Major Project Restructure and efficiency related changes#2083menasheh wants to merge 8 commits intojmoenig:masterfrom
menasheh wants to merge 8 commits intojmoenig:masterfrom
Conversation
Collaborator
|
Following the discussion in #1863, I'm going to close this. While I very much love directories and smaller files, others don't have an interest in that right now. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Introduction
Snap! is a repository of massive javascript files. According to codeclimate, it's "maintainability" is two years, with over 1000 code smell issues and 600 duplication issues (probably translation files). I want to do to snap something similar to what I did to morphic in effort to make it a more enjoyable codebase to work with, and to make it more extensible.
Goals
Personally, I want to make a block-based programming environment for TI calculators. I'm sure there are other people with other use case ideas as well. Technically, I could just fork now and do my own thing from here on. However, I want my project to be able to benefit from future improvements to Snap! without my manually copying code from Snap's several thousand line files to the smaller ones I'd break them down into in effort to both make the project more maintainable and learn about all of it's pieces. If Snap! were a little more modular, with some of the different pieces in different repositories, each published as npm packages and used as dependencies within the other projects as appropriate, that would make it much easier to make another project extending only certain parts of snap and not others. It also makes sense to give back to the Snap! codebase, as it will be a giant upon which my project will stand to reach further. So I'm focusing on things which will benefit both projects first, by trying to make the base project more extensible and maintainable.
For maintainability, I'd want to separate each library into different files and bundle them all together to create the final product, a single file,
snap.min.js. It's a bit complicated, so I'll discuss this more amongst the changes.Changes
Here's what's been changed so far in this PR:
Project Structure
Everything in the top level directory of a project leads to a big mess of files, especially with all the
lang-*.jsfiles. It was hard to find what you're looking for in that awesome pile of code. Here's my proposed system:docsfolder for documentation and help, as well as assorted .txt files that found their way into the repo.distfolder for generated files such assnap.js,snap.min.jsmediafolder forlibs,examples,sounds, and other mediasrcfolder for sourcesrc/langfolder for language filespackage.jsonfor package configuration (see Package Management)gulpfile.jsfor managing build scripts. (See Build Tool)Package Management
Morphic.js has it's own repository, so why should its code be duplicated in this one? We can solve this redundancy problem by using npm, the node package manager. If you don't already have nodejs and npm, you can download them here. (npm comes with nodejs.) From what I understand, these tools are ubiquitous in the javascript development world. In
package.jsonwe includemorphic-guiandfilesaveras dependencies. We can get rid of those respective files in snap's source control, but download them and store them in the gitignorednode_modulesfolder automatically by usingnpm installin Snap!'s main directory. There might be more modules we could use. The sha512, for example, seems to be from another module, but I don't think the currently available module necessarily supports the same functions in the one Snap has.Build Tool
Prior to this PR, snap's main file made 16 javascript requests to load each of the javascript files.
I chose gulp as a build tool for the project. Although webpack seems to be a favorite for a lot of people, it is a module bundler first and a build tool second. I didn't want the webpack wrapper around the files, I just want to concatenate files together and minify the output. I was unable to get webpack to do that, and able to get gulp to do that.
To use gulp, you need to have NodeJS installed. Run
npm i -g gulp-clianywhere andnpm iin this project directory.With the tasks I defined in gulpfile.js, we're able to accomplish various tasks:
gulp scriptswill buildsnap.js,snap.min.js, andsnap.min.js.mapand put them in thedistfolder.gulp imageswill minify any image files found in or under themediafolder.gulp lintwill run jshint on our code. (It's another javascript linter, it has a lot of output at the moment.)gulp watchwill watch thesrcfiles for changes and rungulp scriptsany time they are changed.And there are tons more possibilities on that front. For example, we could add automatic browser reloading when code is changed, some form of unit tests, or a translation-related script, to the gulpfile in the future.
The method of getting the required dependencies included in the file generated by gulp is a working but slightly messy method, as it lists each file specifically in order in the
gulpfile, even for dependencies such asmorphic.js. But, it works.Other efficiency changes
index.htmlwas an empty page redirecting tosnap.html. I thought that was inefficient, so I renamedsnap.htmltoindex.html.gulp imagestask). That saved around 20% of the space the example images were occupying.Import toolsmenu option to use the equivalent file inmedia/libsHope this helps!