This repository has been archived by the owner on Dec 30, 2020. It is now read-only.
Test PR #21
Open
Test PR #21
Conversation
This file contains 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
The project is modernized in patches, almost like the maintainers are afraid to break anything. It's using Babel 7, with Stage Zero preset (meaning *all* of the latest JS features are being transpiled), but not taking advantage of a lot of those features (cf the `events.jsx` file; it makes more sense to do its handlers object as a Map/WeakMap than a raw object, especially if it'll get transpiled down to that anyway). Other features added include a better `.gitignore` file with Node in mind, an EditorConfig file so that editing environs can be unified (with the appropriate plugins, see <https://editorconfig.org> for details), ESLint for any new mistakes that may pop up, Prettier for unified code formatting (feel free to modify it in a `.prettierrc` file, btw), `husky` and `lint-staged` for running Prettier against the modified files (hopefully all files get touched so the codebase ends up unified in appearance), Lodash as an awesome utility kit, and Istanbul and NYC for code coverage. Code coverage was surprisingly high for a project that wasn't tracking it before: just on the initial run of `yarn coverage`, it was already nearly 72 percent. The file with the lowest coverage was `events.jsx`, which is what made me open the file in the first place. Another file to check is the `NodeHistoryAPI`, which was almost as low. So, the to-list is as follows (for me, anyway): [ ] Bring code coverage to over 95% [ ] Bring code coverage to over 75% [ ] Bring code coverage to over 80% [ ] Bring code coverage to over 85% [ ] Bring code coverage to over 90% [ ] Check the code for more modernization opportunities [ ] Try to enhance the code's FP style [ ] Try to reduce any mutation seen (haven't looked through all the code yet, though) [ ] Further modernization [ ] Aim for 100% code coverage? Signed-off-by: Jonathan Sifuentes <jayands2k11@ymail.com>
I've fully integrated the code coverage suite into the cycle. Or rather, I _had_ done so. I was unable to make a push because of it, initially; after doing so, I fully intend to reintroduce it in the cycle. Husky allows for a "prepush" key in the "scripts" that gets run whenever you go to `git push`. Only a clean exit will allow you to successfully continue. Since the branch percentage is still in the low 50s, I couldn't get it to actually let me through. So the "prepush" script is actually supposed to read `npm run coverage && lint-staged`. That said, all the other code coverage is near 80%. Also, I needed flow for a few files (including some that I then stripped it out of, most notably the `RouteHistory` that has decorators, because flow didn't like them), and the `flow-typed/npm` cache in specific is ignored, so if type declarations are written, they can go in the top-level `flow-typed` folder and will be used properly. To get the type definitions, run the script "types". I use the lodash plugin for eslint because it knows all the tricks that I don't; for example `_.compact` instad of filtering on a boolean value. The recommended extensions are things that I think will enhance the coding experience in VSCode. All of them are, of course, optional, but I would implore other users to at _least_ install the EditorConfig one, as that's what makes `.editorconfig` files useful, is people actually enacting the rules imposed. I'm sure I've forgotten more that I've added/modified (c.f the code coverage output), but that seems like a decent summary. Signed-off-by: Jonathan Sifuentes <jayands2k11@ymail.com>
I'm comprimising the code coverage levels so that I can actually commit the `package.json` the way I would like to actually commit it. Furthermore, since the code is going to be moving to `react-router` (c.f. #2), I personally don't feel like it's a great idea to write any more tests that will probably have to be discarded (or at least heavily rewritten) anyway until after the migration. Signed-off-by: Jonathan Sifuentes <jayands2k11@ymail.com>
Travis CI uses NVM to [specify which versions][t-vers] it uses. I don't know if it's exactly reasonable to be expected to support versions that Node.js itself [doesn't support][lts-schedule], so this modifies that. Furthermore, since there's a `yarn.lock` file, Travis can cache the yarn packages leading to faster builds/tests. [lts-schedule]: https://github.com/nodejs/Release#lts_schedule [t-vers]: https://docs.travis-ci.com/user/languages/javascript-with-nodejs/#Specifying-Node.js-versions Signed-off-by: Jonathan Sifuentes <jayands2k11@ymail.com>
This commit adds a `config/rollup` folder, and in it, a common config to be shared and the two configurations to be ran by rollup: a development environment one which does not minify the output, and a production one that does. I did it that way to avoid duplication, since the majority of the settings are the same for both. As a result, chances are the file you want to look at/edit is the file `rollup.config.common.js` for the most pertinent settings. I also realized that I forgot to include a few things in the `.eslintrc.js` and that I forgot to include my usual `lint` and `lint:fix` scripts. This patch also fixes that, so linting can be done on the command line as well. Signed-off-by: Jonathan Sifuentes <jayands2k11@ymail.com>
Rollup's settings of `modules: false, useBuiltins: false` for the env preset breaks the build for mocha, so I just copied the settings to the test environment and removed them. I'm glad I caught it with the prepush hook, though; it would have been bad to have travis fail like that. Signed-off-by: Jonathan Sifuentes <jayands2k11@ymail.com>
Rollup.js (fixes #4)
Remove jsx extension
- Added code styles to Props tags to make them contiguous with the example code. - Created some code blocks for import statements
Code md changes in readme
Upgrade vulnerable packages
Small grammatical changes to the README.md file for easier reading by the user.
Create gettingStarted.md
Update README.md
Updated Docs and README.md
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Test PR