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: Frontend scripts and Tailwind accounting for products/ #27696

Merged
merged 2 commits into from
Jan 21, 2025

Conversation

skoob13
Copy link
Contributor

@skoob13 skoob13 commented Jan 20, 2025

Problem

Frontend scripts don't work for the /products/** dir.

Changes

  • Fix jest, eslint, stylelint, and typegen.
  • FIx lint-staged.
  • Fix tailwind.

I might've missed some other scripts.

Does this work well for both Cloud and self-hosted?

Yes

How did you test this code?

Locally

@skoob13 skoob13 requested review from mariusandra and Twixes January 20, 2025 17:00
Copy link
Collaborator

@mariusandra mariusandra left a comment

Choose a reason for hiding this comment

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

Whoops. Thanks for this!

@skoob13 skoob13 enabled auto-merge (squash) January 20, 2025 17:10
package.json Outdated
"prettier --write"
],
"products/**/frontend/**/*.{js,jsx,mjs,ts,tsx}": [
"eslint --cache products/**/frontend/**/* -c .eslintrc.js --fix",
"prettier --write products"
Copy link
Member

@Twixes Twixes Jan 20, 2025

Choose a reason for hiding this comment

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

This runs prettier on the whole products/ directory for each file changed under products/**/frontend/**/... – which is why this PR contains changes to a README.md, which wasn't otherwise changed.

Suggested change
"prettier --write products"
"prettier --write"

Curious why we need a separate Eslint cache BTW, if the original formatting command here didn't specify the cache path?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We don't need a separate cache. Lemme fix.

Copy link
Contributor

github-actions bot commented Jan 20, 2025

Size Change: +5 B (0%)

Total Size: 1.16 MB

ℹ️ View Unchanged
Filename Size Change
frontend/dist/toolbar.js 1.16 MB +5 B (0%)

compressed-size-action

@skoob13 skoob13 requested a review from Twixes January 20, 2025 17:48
@posthog-bot
Copy link
Contributor

📸 UI snapshots have been updated

1 snapshot changes in total. 0 added, 1 modified, 0 deleted:

  • chromium: 0 added, 1 modified, 0 deleted (diff for shard 1)
  • webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

@skoob13 skoob13 disabled auto-merge January 20, 2025 18:49
package.json Outdated
"eslint -c .eslintrc.js --fix",
"prettier --write"
"eslint -c .eslintrc.js --fix cypress/**/*",
"prettier --write \"cypress/**/*.{js,mjs,ts,tsx,json,yaml,yml,css,scss}\""
Copy link
Member

Choose a reason for hiding this comment

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

This specification of path also is unwanted, since lint-staged already provides the path of every changed file. Same in eslint above

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks. I've fixed paths.

@skoob13 skoob13 requested a review from Twixes January 21, 2025 09:18
@skoob13 skoob13 force-pushed the chore/products-folder-scripts branch from 90e8d80 to b4c35ae Compare January 21, 2025 09:27
Copy link
Member

@Twixes Twixes left a comment

Choose a reason for hiding this comment

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

Nice, thanks

@Twixes Twixes changed the title chore: frontend scripts for the products dir fix: Frontend scripts and Tailwind accounting for products/ Jan 21, 2025
@Twixes Twixes merged commit e5618dc into master Jan 21, 2025
101 checks passed
@Twixes Twixes deleted the chore/products-folder-scripts branch January 21, 2025 11:20
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.

4 participants