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: exclude tabler/icons-react from optimizeDeps to avoid crashing performance CI #6621

Merged
merged 1 commit into from
Aug 16, 2024

Conversation

Nabhag8848
Copy link
Contributor

@Nabhag8848 Nabhag8848 commented Aug 14, 2024

ISSUE (Warning)

Things needed to investigate :

  • Can we exclude @tabler-icons from Babel / wyw-in-js / the vite config of Storybook ?

Yes we can

  • Is there a reproducible difference between non-Linaria components loading time and Linaria components loading times ?

Couldn't find anything related to this, yet. but changes fix the problem.

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

This pull request addresses slow loading times and potential crashes in Storybook by modifying the Vite configuration to exclude '@tabler/icons-react' from dependency optimization.

  • Added optimizeDeps: { exclude: ['@tabler/icons-react'] } in packages/twenty-front/vite.config.ts
  • This change aims to reduce load times for stories, particularly those using Linaria components
  • Addresses the Babel deoptimization warning for large files (>500KB) mentioned in issue Problem with Babel, wyw-in-js and Linaria with Storybook #6437
  • May help prevent performance CI crashes related to Storybook loading
  • Further investigation needed to compare loading times between Linaria and non-Linaria components

1 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings

@Nabhag8848 Nabhag8848 changed the title fix: exclude tabler/icons-react from optimizeDeps fix: exclude tabler/icons-react from optimizeDeps to avoid crashing performance CI Aug 14, 2024
@Nabhag8848
Copy link
Contributor Author

@lucasbordeau

@lucasbordeau lucasbordeau self-assigned this Aug 14, 2024
Copy link
Contributor

@lucasbordeau lucasbordeau left a comment

Choose a reason for hiding this comment

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

Nice !

@lucasbordeau lucasbordeau merged commit 20d8475 into twentyhq:main Aug 16, 2024
9 of 11 checks passed
Copy link

Thanks @Nabhag8848 for your contribution!
This marks your 0th PR on the repo. You're top 0% of all our contributors 🎉
See contributor page - Share on LinkedIn - Share on Twitter

Contributions

BOHEUS added a commit to BOHEUS/twenty that referenced this pull request Aug 16, 2024
BOHEUS added a commit to BOHEUS/twenty that referenced this pull request Aug 17, 2024
@BOHEUS BOHEUS mentioned this pull request Aug 17, 2024
BOHEUS added a commit to BOHEUS/twenty that referenced this pull request Aug 21, 2024
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.

Problem with Babel, wyw-in-js and Linaria with Storybook
2 participants