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

Auto format CSS and JS syntax #159

Open
ascott1 opened this issue Feb 13, 2018 · 2 comments
Open

Auto format CSS and JS syntax #159

ascott1 opened this issue Feb 13, 2018 · 2 comments

Comments

@ascott1
Copy link
Member

ascott1 commented Feb 13, 2018

Related to the discussion in cfgov-refresh #666 🤘

Newer tooling, such as Prettier allows for automated syntax formatting of JS and CSS. This reduces the need for syntax checks in code reviews and allows the reviews to be focused on the code itself. To quote @jimmynotjim:

being able to offload nonsense like this from each of our mental weights would be 💯

Prettier can be used in the following ways:

  • CLI tooling/npm scripts
  • Editor plugins
  • As a pre-commit/push hook with something like husky

The one potential blocker to using something like Prettier is that it has a limited configuration. In many ways this is a feature, but there may be ways that those differ from our current documented standards. The biggest one to jump out at me is our use of spaces within parens ( space ) vs (space).

A few questions for discussion:

  • Are we in favor of doing this? If so is Prettier the right tool for us?
  • Are there other differences from our documented standards? If so, are they worth updating for the ease of use Prettier provides?
  • What is the right way to integrate these into the team and individual projects? (my recommendation: encourage use of the editor extensions and set up pre-commit hooks)
@jimmynotjim
Copy link
Contributor

Progress so far. Tested Prettier and a combo of ESlint and Stylelint with --fix flags. It seems neither tool can be added to our repos without a bit of work. Both Prettier and Stylelint choke on a few of our mixins, especially those that contain any !important rules (that may say something about how we're building mixins, but we'll leave that for another discussion). Prettier has an option to only test the files that are ready for testing using --require pragma, I haven't found a similar option with ESlint. I'm going to keep moving forward with Prettier and see if I can create a baseline of formatted files and we can postpone formatting for those it's choking on till we have time to dig into them.

@jimmynotjim
Copy link
Contributor

Got Prettier into a working order via a combo of pragma and ignore comments where it was tripping up or we didn't want it to run https://github.com/cfpb/capital-framework/compare/prettier?expand=1

This looks better than anything I could get working with ESLint and StyleLint --fix alone. The nice thing is now ESLint and StyleLint can concentrate on real format issues. I know @anselmbradford is out, so we'll wait on a final decision until he returns on Wed, but are there other concerns from the @cfpb/cfgov-frontends?

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

No branches or pull requests

2 participants