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

Turn on the tailwindcss/classnames-order rule #3415

Open
jeoffreyfischer opened this issue Dec 17, 2024 · 4 comments · May be fixed by #3417
Open

Turn on the tailwindcss/classnames-order rule #3415

jeoffreyfischer opened this issue Dec 17, 2024 · 4 comments · May be fixed by #3417

Comments

@jeoffreyfischer
Copy link
Member

jeoffreyfischer commented Dec 17, 2024

Related to #3404

Description
Investigate whether we can reopen the tailwindcss/classnames-order rule

Acceptance Criteria

  • tailwindcss/classnames-order rule is turned back on
@jeoffreyfischer jeoffreyfischer converted this from a draft issue Dec 17, 2024
@Calinator444
Copy link
Member

Our eslint version may not be compatible with node v22.12.0 and upgrading eslint is blocked until we upgrade to node v15.

vercel/next.js#64409 (comment)

@babakamyljanovssw babakamyljanovssw self-assigned this Dec 17, 2024
@babakamyljanovssw babakamyljanovssw moved this from 🔖 Backlog - Ready to 🏗 In progress in SSW.Website Dec 17, 2024
@babakamyljanovssw
Copy link
Member

babakamyljanovssw commented Dec 18, 2024

cc: @Calinator444 @jeoffreyfischer @isaaclombardssw

Hi @wicksipedia

I enabled tailwindcss/classnames-order rule and the pipeline started to fail again.

Tested pr-lint-code.yml workflow with different node LTS versions.

  • with node 22.12.0 (LTS): ❌ it has failed with several classnames order errors, even the order of classnames is correct
  • node 20.18.1 (LTS): ✅ completed successfully

As Caleb also mentioned, the eslint 8.57.1 may not be compatible with node 22.12.0 we use in pipeline.

There are two options:

  • Upgrade eslint to version 9, but we also need to upgrade NextJS version
  • Or we can use node 20.18.1 (LTS) only in this pr-lint-code.yml workflow for now

@jeoffreyfischer
Copy link
Member Author

As per our conversation with @wicksipedia , we will raise this issue to GitHub
@babakamyljanovssw please create a repo that shows the bug

@babakamyljanovssw
Copy link
Member

babakamyljanovssw commented Dec 20, 2024

cc: @wicksipedia @jeoffreyfischer

I've created nextjs 14.2.15 project - https://github.com/babakamyljanovssw/nextjs-eslint
Added eslint, prettier ... packages, then configured eslintrc.json and .prettierrc.

But, I wasn't able to reproduce the issue from SSW.Website.

For testing I used the below classnames from SSW.Website:
prose prose-h1:!my-0 prose-h1:!pt-4 prose-h3:!mt-0 prose-img:!my-0 flex-1 pt-4 - link to SSW.Website location for this class

Testing in new project:

  • Copied above classnames to file
  • Ran pnpm lint, no errors and no VS code red line warnings

Testing in SSW.Website project:

  • VS Code was initially showing visual red line warning by eslint, but running pnpm lint didn't gave any classname order error
  • But saving the file changed to the previous order (which is not correct) automatically

Also, in the new project using node 20 and node 22 gave the same result, but on SSW.Website node 20 contradicting node 22 suggestion order and node 22 was contradicting node 20 suggested order.

I think the problem might be on SSW.Website project configuration or tailwind configuration. I couldn't figure out yet the underlying cause yet.

So I didn't change the node version in the pipeline. I just ran pnpm lint-fix to fix all classnames order errors.
The PR is ready for review - #3417

@babakamyljanovssw babakamyljanovssw removed their assignment Dec 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: 🏗 In progress
Development

Successfully merging a pull request may close this issue.

3 participants