-
Notifications
You must be signed in to change notification settings - Fork 526
Description
Hey code.org team! I'm a senior developer interested in contributing to your org and thought it might be a good step just starting with a few small refactors to familiarize myself with the codebase.
I've read the contribution guide, but I'd just like some clarifications before I invest a bit of my time in to contributing. With that in mind could you please let me know what's off-limits / permissible for an external contributor? What I had in mind was starting with the following:
1. Typescript migration for individual files
- I noticed that since the codebase is quite mature, there's a lot of JSX/JS files that could be individually moved over to Typescript. At the very minimum if there isn't a build step for TS I could at least enable the typechecking component and add the proper JSDoc typings.
- Likewise, ideally we'd migrate away from using
prop-typesruntime checking to typescript type checks. - The Redux dispatch function seems to be a good candidate for typing which seems to be a project in itself and may just expose a few bugs:
code-dot-org/apps/src/util/reduxHooks.ts
Lines 11 to 14 in f59eff6
export type AppDispatch = ThunkDispatch<RootState, undefined, AnyAction> & Dispatch<AnyAction>; export const useAppDispatch: () => AppDispatch = useDispatch;
2. Cleanup/consolidation of duplicate libraries
I noticed that the codebase uses multiple libraries for the same purposes that could be consolidated:
e.g. for CSV file generation, we're installing three packages that essentially fulfill the same purpose:
code-dot-org/apps/package.json
Line 300 in f59eff6
| "papaparse": "^5.4.1", |
code-dot-org/apps/package.json
Line 318 in f59eff6
| "react-csv": "^2.0.3", |
code-dot-org/apps/package.json
Line 365 in f59eff6
| "save-csv": "^4.0.6", |
3. Fixing some anti-patterns
I did notice a number of instances where the way useEffect is being called isn't strict mode compliant. For example the data fetching in useEffects are done without abort signals which can cause race conditions.
4. Dockerizing dev environment
Setting up this repo locally was a bit painful (I didn't really want to install & run mysql / redis on my machine), are y'all open to a docker-compose config for running the apps in docker container stack for development?
5. Security fixes
I noticed that the security page mentions a Bug Bounty, however I'm guessing PRs are a no-no and I should stick to only using the bug bounty right? Oh, fwiw it could be beneficial adding a SECURITY.md to the repo (adds the security tab to github) and /security.txt endpoint.
Apologies for the barrage of questions, and thank you in advance!
- James