Document and unify pre-commit hooks for Python and JavaScript
Problem
The project's pre-commit setup is not documented, causing confusion and CI failures (e.g., in PR #906). Checks are not enforced locally for all contributors.
Suggested Solution
- Unify Hooks: Extend the existing
.pre-commit-config.yamlto include JavaScript checks (ESLint, Prettier) alongside the Python checks (Ruff). - Document Setup: Create a
CONTRIBUTING.mdor updateDEVELOP.mdto instruct contributors on how to install and use the hooks withuv run pre-commit install.
Indeed we should add a section in DEVELOP.md suggesting to use uv run pre-commit install to install pre-commit hooks locally.
- Unify Hooks: Extend the existing
.pre-commit-config.yamlto include JavaScript checks (ESLint, Prettier) alongside the Python checks (Ruff).
Pre-commit is a fully "static" lint approach, meaning that it has access to your code, but no dependencies. Pre-commit runs in isolated environments with only your code and the thing being run in that step.
This is why on the Python side, code format and linting is applied but not type checking, because that needs access to installed dependencies to work correctly.
We could probably add TS code formatting to pre-commit, but I assume we'd run into the same issue for type checking in TS that we do in Python.
Pre-commit is meant as an easy way to do a limited number of static checks, not all checks that might be run on CI
Might be worth switching to lefthook, which we use in veda-ui to lint and type check. It’s auto-installed on yarn install and pretty fast. Looks like it can be set up to run Python checks too.
As I understand it, this problem is universal regardless of what tool you use to run pre-commit checks.
Assuming that lefthook uses isolated environments, it won't have Python dependencies available when type checking, which means that type checking will be wrong. (The same issue exists for Typescript, because if you don't have dependencies installed when you run tsc, it won't catch any bugs when you interface with dependencies).
And if lefthook didn't use an isolated environment, then it would give different results whether you run it locally or in CI, which would be equally bad.
We should add information to DEVELOP.md but I don't see anything else actionable here.
It’s possible to guarantee exact dependency versions for both Python and JavaScript with yarn install --frozen-lockfile and uv sync --frozen, ensuring lefthook runs in the same environment locally and in CI.
I see; so lefthook doesn't use isolated environments like pre-commit does.
I don't have a strong opinion here. I'd be fine with just documenting the existing state. But also open to changing the pre-commit provider if you want to implement that. I just want pre-commit checks to be fast (<1-2 seconds) so I don't want to e.g. run all of tsc on pre-commit (but maybe there are newer, faster type checking implementations?).
(We ran really slow checks like tsc on deck.gl/kepler.gl and it meant that a commit took like 40 seconds to run, so I ended up just always force-pushing my commits)