feature/pre commit

[joshua-dean]

Formatting and Linting Hooks

Description

Added:

• ESLint
• prettier
• husky (which uses lint-staged for nice terminal outputs)
• .editorconfig file for uniform configuration of things like indent sizes

I expect this might remain open for a bit longer than other PRs, so I'm going to forego constantly merging the latest main until we've settled on a good set of rules.

PR Checklist

• [x] Merged latest main
• [x] Version number in package.json has been bumped since last release
• [x] Version numbers match between package package.json and src/version.js
• [x] Ran npm install and npm run build AFTER bumping the version number
• [x] Updated documentation if necessary (currently just in api_spec.md)
• [x] Added changes to changelog.md

Breaking API Changes

nope

[joshua-dean]

To get started I made a couple of commits on src/index.js and src/toolbox.ts, so we could see linting/formatting on both javascript and typescript files. I separated prettier, eslint complaints, and eslint auto-fixes (when applicable) into separate commits so you could inspect them separately.

The only rule I've set is 4 spaces for indentation, which (mostly) matches what we've been doing. For many of the eslint complaints, I left TODOs if I was concerned about harming functionality; we can always fix those later.

As a side note, prettier's philosophy is to not be very configurable, so if we find anything we hate that we can't configure, we should can it sooner rather than later.

@TrevorBurgoyne @csolbs24 please complain as much as possible - I'd prefer to get a good set of rules now so we don't need massive diffs down the road.

[joshua-dean]

i think eslint has both an import sorter and a jsdoc requirement option so id say we turn both those on. maybe the jsdoc can just be a warning for now?

Yeah, I toggled sort-imports on in b4be8bc, and fixed all the complaints in 2095c8a. It doesn't auto-fix everything, and the default settings are very particular about ordering. We might want to adjust some of the rules, or consider using a sorting plugin that does all the heavy lifting for us.

[joshua-dean]

JSDoc enforcement is now done via the eslint-plugin-jsdoc plugin. It has some conflicts with our typescript packages. Seeing as we're going to all-typescript anyways (#180), should we just pick a plugin that enforces TSDoc and let that all shake out when #180 is solved?

[TrevorBurgoyne]

i think eslint has both an import sorter and a jsdoc requirement option so id say we turn both those on. maybe the jsdoc can just be a warning for now?

Yeah, I toggled sort-imports on in b4be8bc, and fixed all the complaints in 2095c8a. It doesn't auto-fix everything, and the default settings are very particular about ordering. We might want to adjust some of the rules, or consider using a sorting plugin that does all the heavy lifting for us.

yeah iirc it likes alphabetical where caps come first, and then multiple imports before single imports. i really don't care much about the rules, the multiples-first can get annoying to manually fix but does look nice. if there is a plugin that moves them automatically like isort that would be lovely

[joshua-dean]

@TrevorBurgoyne I went through the rules alphabetically (through F) and added what I thought was logical in 5506824. The @stylistic formatting is in 39819a3. Much more minimal changes. I'll peruse the rest of the rules for anything notable.

[joshua-dean]

@TrevorBurgoyne I went through the rules alphabetically (through F) and added what I thought was logical in 5506824. The @stylistic formatting is in 39819a3. Much more minimal changes. I'll peruse the rest of the rules for anything notable.

Actually scratch that, they have presets that I wasn't leveraging, I'm going to change to using those.

[joshua-dean]

OK I used the shared config in e587692, d6cab45, and f5f7680, and then applied the new formatting in c2a2f7b.

[joshua-dean]

@TrevorBurgoyne does this look good to go forward with? Should I blindly apply to the whole codebase to see if there's any disasters?

[TrevorBurgoyne]

@TrevorBurgoyne does this look good to go forward with? Should I blindly apply to the whole codebase to see if there's any disasters?

yeah i like all the rules... so feel free to send it 🚀

[joshua-dean]

yeah i like all the rules... so feel free to send it 🚀

OK, currently working on this. I did the automatic fixes in f770571.

I did some manual fixes for the no-unsafe-function-type rule in 7178400, and it seemed like the best way to handle this case was generics (typescript docs, w3schools docs).

I've used generics in python, but this is my first time in typescript. I did them for the time_function as that wraps an arbitrary function. Let me know if you think the set_local_storage_item and get_local_storage_item are a good case for them too.

[joshua-dean]

@TrevorBurgoyne I've finished resolving ESLint issues.

I left a lot of TODOs, a lot of any types are related to #131 so aren't really worth chopping up now. We currently don't have anything for import sorting or JSDoc/TSDoc enforcement. I can try to sort those out in this PR if we want, or add them to #103 and sort them out later.

[TrevorBurgoyne]

I can try to sort those out in this PR if we want, or add them to #103 and sort them out later.

Yeah i have to scroll down far enough on this pr as is, id say split those out for later 👍

[joshua-dean]

Yeah i have to scroll down far enough on this pr as is, id say split those out for later 👍

I added a note on #103. There aren't any other PRs that look like they're close to merging, so I'll get the latest main merged and get this up-to-date.

[joshua-dean]

very close, a couple notes. also i think we should add a GHA as a linting check to make sure ppl actually have their pre commit hooks setup correctly

Opened #192 to track this