Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(next): version 5.0.0 with lefthook #151

Closed
wants to merge 1 commit into from
Closed

feat(next): version 5.0.0 with lefthook #151

wants to merge 1 commit into from

Conversation

varl
Copy link
Contributor

@varl varl commented Oct 29, 2019

Polyglot Git hook management

I have been playing around with using Lefthook for managing our Git hooks for a while now, to replace Husky and LintStaged. Those two have been great for us for as long as d2-style has been a frontend-first tool.

The idea has always been to support more than just JavaScript, and while it has been possible to do so, configuration and setup for Husky and LintStaged has been difficult, forcing us to use a validate command which sets up everything for LintStaged -- but that only works for JavaScript.

So here's a working implementation of Git hook management that works for frontend and backend that does away with most of our custom code with how we integrate tools like commitlint, prettier, eslint, which in turn makes it a lot easier to reason about and add new tools that do not have a NodeJS API.

Like all things, this comes at a cost and requires a workflow change for us as d2-style becomes less aggressive in how it goes about its business.

Trade-offs

➕ Pros! ➖ Cons!
Faster! No partial file support, which is a regression in functionality for us
Fewer deps! Uses the output from the underlying commands and drops our custom report format
Simpler! Does not run all tools if one fails (abort early)
Works better for polyglot environments
Windows support!
Supports Docker commands!

Workflow change

Regarding the regression in functionality (losing automatically fixing and staging hunks in a pre-commit hook), the intended workflow is slightly different.

  • Today: Automatically fix everything we can and stage the results before committing.
  • Future: Run the checks yet do not automatically fix, allow user to reformat manually and stash/lint/add and commit the results.

It is only important if we want to automatically apply any code formatting when we commit files, and there is an alternative workflow where we check our files for code style before we commit, and abort the commit if the code style fails.

This means that a user will have to run d2-style js apply --staged manually and add the changes she wants.

Simpler development

Compare the files in tools:

  • src/tools/js/prettier.js with the replacement src/tools/prettier.js.
  • src/tools/js/eslint.js with src/tools/eslint.js.

Adding new tools in the same style is trivial, where before we relied on NodeJS API bindings to wrangle everything to how we wanted it internally.

The new structure is a lot easier to grok:

src/
├── commands
│   ├── commit.js
│   ├── javascript.js
│   ├── setup.js
│   └── structured-text.js
├── commands.js
├── index.js
├── tools
│   ├── commitlint.js
│   ├── eslint.js
│   ├── lefthook.js
│   └── prettier.js
└── utils
    ├── config.js
    ├── files.js
    ├── groups.js
    ├── paths.js
    └── run.js

New features

Global installation

Since we don't rely on Husky to install the Git hooks on a postinstall we do not need to install d2-style as a dependency in repositories that might not want a package.json file and running "{npm,yarn} install` in their workflow.

If d2 CLI and Lefthook is available globally, then this is enough to install the hooks in any repository:

d2 style setup git/hooks-backend

This creates a lefthook.yml file and installs the commit hooks in .git/hooks.

Structured text

Use Prettier to check/apply format rules for various types of structured text:

lint-structured-text:
    glob: '*.{yaml,yml,md,markdown,json,html,htm}'
    run: npx d2-style structured-text check {staged_files}

project/react Setup

Generate ESLint configuration for a React project:

d2-style setup project/react

ESLint config includes recommended rules

We tried working from bottom-up principles in previous versions, adding more rules as we found them useful, but that work has died off and multiple repos are now (probably wisely) adding eslint:recommended to the .eslintrc.js configuration file. This centralises and adds it together with the eslint-config-prettier to avoid any conflicts between the two.

@amcgee amcgee self-requested a review November 1, 2019 09:55
@varl varl changed the title feat(hooks): replace husky+lint-staged with lefthook feat(next): version 5.0.0 of cli-style Nov 4, 2019
@varl varl force-pushed the lefthook branch 2 times, most recently from 825e03f to 919bf8a Compare November 4, 2019 19:45
@varl
Copy link
Contributor Author

varl commented Nov 4, 2019

There is a published beta that can be installed from the dist-tag "lefthook";

yarn add --dev @dhis2/cli-style@lefthook

We want to make sure that it works when installing it and this is the most robust way that I can think of to test it.

BREAKING CHANGE: This rips out Huksy+LintStaged and replaces it with
another Git hook manager called Lefthook.
@varl varl changed the title feat(next): version 5.0.0 of cli-style feat(next): version 5.0.0 with lefthook Nov 4, 2019
@varl
Copy link
Contributor Author

varl commented Nov 4, 2019

After having used the lefthook-beta for a while there is a very real drawback -- it does not support monorepos at all (yet).

I have branched out to another branch that saves the features and refactoring, but rolls back the change to Lefthook from Husky.

I have not re-added lint-staged as that's where a lot of the "mess" came from.

@varl varl closed this Nov 4, 2019
@varl varl deleted the lefthook branch October 28, 2020 11:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant