Skip to content

Conversation

@jgravois
Copy link
Member

@jgravois jgravois commented Dec 2, 2025

resolves #541

theoretically it should be possible to get prettier + plugins configured to run via pre-commit, but i haven't found any joy getting that setup with @shopify/prettier-plugin-liquid yet (CIL for more notes about that).

prettier/pre-commit#16

as for the formatting changes... i figured it made sense to start with a single file as a conversation starter.


alternatives explored

https://shopify.dev/docs/storefronts/themes/tools/shopify-liquid-vscode#code-formatting

i could only get ☝️ to autoformat .liquid files. 🤷

@netlify
Copy link

netlify bot commented Dec 2, 2025

Deploy Preview for cal-itp-website ready!

Name Link
🔨 Latest commit f273bd0
🔍 Latest deploy log https://app.netlify.com/projects/cal-itp-website/deploys/6931bc5374047d0008fc1789
😎 Deploy Preview https://deploy-preview-537--cal-itp-website.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@jgravois jgravois force-pushed the chore/yank-djlint branch 2 times, most recently from 69dbed8 to 8bc5357 Compare December 3, 2025 00:37
@jgravois jgravois changed the title chore: use prettier to format instead of djlint + shopify-liquid chore: use shopify-liquid VS code plugin to autoformat html Dec 3, 2025
"extensions": [
"eamodio.gitlens",
"esbenp.prettier-vscode",
"prettier.prettier-vscode",
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

- id: djlint-reformat-jinja
files: \.html$
types_or: ["html"]

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i futzed around for longer than i probably ought this morning trying to wire up prettier + @shopify/prettier-plugin-liquid to run as a pre-commit hook too.

the configuration below doesn't error, but it results in formatting that does not match what is generated in VS Code (even when i made sure to use prettier@2 or prettier@3 consistently)

- repo: https://github.com/pre-commit/mirrors-prettier
    rev: v2.7.1
    hooks:
      - id: prettier
        name: prettier + plugin-liquid
        additional_dependencies:
          - [email protected]
          - "@shopify/[email protected]"
        args:
          - --plugin=@shopify/prettier-plugin-liquid
        types_or: [css, html]
Image

i'm in favor of merging this PR as-is if we prefer the formatting that is produced in VS Code and just accepting that changes made via the github UI will no longer be formatted automatically.

i'm interested in hearing other opinions though. it wouldn't hurt my feelings to just close this PR and let sleeping dogs lie either.

@jgravois jgravois marked this pull request as ready for review December 3, 2025 20:26
@jgravois jgravois requested a review from a team as a code owner December 3, 2025 20:26
@thekaveman
Copy link
Member

RE:

i could only get ☝️ to autoformat .liquid files. 🤷

Did you try updating the file extension specific formatter config? E.g. we have an entry for liquid files (and I don't think we actually use that extension anywhere), while also telling VS Code it should treat .html files as liquid

Sounds like you messed around with this a bit, just wanted to ask.

@jgravois
Copy link
Member Author

jgravois commented Dec 3, 2025

Did you try updating the file extension specific formatter config?

i did indeed get that bit working and updated the PR description and subsequent comments to clarify where this PR stands now.

@thekaveman
Copy link
Member

thekaveman commented Dec 3, 2025

Sorry if I wasn't clear; I meant in trying that other option: https://shopify.dev/docs/storefronts/themes/tools/shopify-liquid-vscode#code-formatting

If you tried adding an explicit html language override / section to tell it to use that formatter. Probably neither here nor there, ignore! 😆

Copy link
Member

@Scotchester Scotchester left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One thing I would definitely like to see before merging this:

I assume we'll want to use the new config to mass-format all our existing files. When doing so, let's confine the formatting to a commit that only does that, then add the commit SHA to .git-blame-ignore-revs?

@jgravois jgravois requested a review from a team as a code owner December 4, 2025 16:52
@jgravois
Copy link
Member Author

jgravois commented Dec 4, 2025

i did a dry-run and discovered two issues in our HTML that needed to be fixed prior to bulk autoformatting. f273bd0

when the runway is clear, now it'll be a cinch to do the actual deed:

mise exec node@24 -- npm install --no-save [email protected]
mise exec node@24 -- npm install --no-save @shopify/[email protected]
mise exec node@24 -- prettier --write "src/**/*.{css,html}"

afterwards i'll commit that change and inject the SHA into a new .git-blame-ignore-revs file in the root of the repo (TIL!)

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.

chore: switch to a more liquid oriented autoformatting tool

5 participants