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

fix(cli): avoid require in prettier config during tailwind setup #10183

Merged
merged 4 commits into from
Mar 11, 2024

Conversation

Josh-Walker-GM
Copy link
Collaborator

@Josh-Walker-GM Josh-Walker-GM commented Mar 10, 2024

Problem
With the recent switch to prettier v3 we have some new constraints on how we write our prettier config. One of these is the need to avoid require statements. The tailwind setup command in the CLI was adding this to the config file and causing runtime errors on commands like lint.

Changes

  1. Switches the setup command to simply insert the name of the plugin. See here as to guidance on this.

    Strings provided to plugins are ultimately passed to import() expression, so you can provide a module/package name, a path, or anything else import() takes.

  2. Regenerated the test project fixture and in so doing it bumped the prettier-plugin-tailwindcss version. You can also see some formatting for graphql tags have altered as a result of the upgrades.

@Josh-Walker-GM Josh-Walker-GM added the release:fix This PR is a fix label Mar 10, 2024
@Josh-Walker-GM Josh-Walker-GM added this to the next-release milestone Mar 10, 2024
@Josh-Walker-GM Josh-Walker-GM self-assigned this Mar 10, 2024
jtoar added a commit that referenced this pull request Mar 10, 2024
@Josh-Walker-GM pointed out some breaking changes I included in the
prettier v3 upgrade (#10179) in
his PR [here](#10183). My goal
with the prettier v3 upgrade was to first do it in a non-breaking way so
that it could be released in the next minor, then come back and include
the breaking changes (mainly around the tailwind setup command). This
should undo the breaking changes so that we can release prettier v3 in
the next minor and redo #10182.
jtoar added a commit that referenced this pull request Mar 10, 2024
@Josh-Walker-GM pointed out some breaking changes I included in the
prettier v3 upgrade (#10179) in
his PR [here](#10183). My goal
with the prettier v3 upgrade was to first do it in a non-breaking way so
that it could be released in the next minor, then come back and include
the breaking changes (mainly around the tailwind setup command). This
should undo the breaking changes so that we can release prettier v3 in
the next minor and redo #10182.
Copy link
Contributor

@jtoar jtoar left a comment

Choose a reason for hiding this comment

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

@Josh-Walker-GM nice find! That saves us a lot of trouble. I thought at first we'd also have to change the file extension to .mjs since module.exports is CJS-only, but that doesn't seem to be the case. Updated this branch based on my change here: #10188.

@jtoar jtoar merged commit 2690968 into main Mar 11, 2024
41 checks passed
@jtoar jtoar deleted the jgmw-fix/tailwindcss-plugin-prettier-v3 branch March 11, 2024 00:35
jtoar added a commit that referenced this pull request Mar 12, 2024
…10183)

**Problem**
With the recent switch to prettier v3 we have some new constraints on
how we write our prettier config. One of these is the need to avoid
`require` statements. The tailwind setup command in the CLI was adding
this to the config file and causing runtime errors on commands like
lint.

**Changes**
1. Switches the setup command to simply insert the name of the plugin.
See [here](https://prettier.io/docs/en/plugins#using-plugins) as to
guidance on this.
> Strings provided to plugins are ultimately passed to [import()
expression](https://nodejs.org/api/esm.html#import-expressions), so you
can provide a module/package name, a path, or anything else import()
takes.
2. Regenerated the test project fixture and in so doing it bumped the
`prettier-plugin-tailwindcss` version. You can also see some formatting
for graphql tags have altered as a result of the upgrades.

---------

Co-authored-by: Dominic Saadi <dominiceliassaadi@gmail.com>
jtoar added a commit that referenced this pull request Mar 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release:fix This PR is a fix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants