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

[2.x] Improve the Vite integration #2060

Merged
merged 30 commits into from
Dec 8, 2024

Conversation

caendesilva
Copy link
Member

@caendesilva caendesilva commented Dec 8, 2024

Targets #2006

  • Refactors code
  • Adds documentation
  • Adds more tests
  • Throws if the path is not a supported extension (Laravel assumes non-CSS is JavaScript... I'd rather be explicit)
  • Removes the internal HYDE_SERVER_VITE environment variable
    • The complexity this adds to microoptimize things is not warranted yet
    • The Vite server configuration already manages adding and removing the hotfile.
    • We need this to print the start message. Instead, it renames the variable to be more clear it's internal Edit: Fixed by just touching the hotfile right away

Copy link

codecov bot commented Dec 8, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (1a261e2) to head (83264ca).
Report is 31 commits behind head on new-asset-system.

Additional details and impacted files
@@                 Coverage Diff                  @@
##             new-asset-system     #2060   +/-   ##
====================================================
  Coverage              100.00%   100.00%           
- Complexity               1932      1937    +5     
====================================================
  Files                     196       196           
  Lines                    5141      5154   +13     
====================================================
+ Hits                     5141      5154   +13     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@caendesilva caendesilva force-pushed the improve-the-vite-integration branch from babe24b to 30b0939 Compare December 8, 2024 15:27
@caendesilva caendesilva force-pushed the improve-the-vite-integration branch from fa73114 to a511976 Compare December 8, 2024 15:29
@caendesilva caendesilva force-pushed the improve-the-vite-integration branch from 40d186d to b048fae Compare December 8, 2024 17:05
Comment on lines +15 to +16
protected const CSS_EXTENSIONS = ['css', 'less', 'sass', 'scss', 'styl', 'stylus', 'pcss', 'postcss'];
protected const JS_EXTENSIONS = ['js', 'jsx', 'ts', 'tsx'];
Copy link
Member Author

Choose a reason for hiding this comment

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

This is seriously overkill, but it doesn't really add any more complexity to add all these. While they might not work unless more media file extensions are registered, adding all these here future proofs things without needing to support customization here.

@caendesilva caendesilva mentioned this pull request Dec 8, 2024
17 tasks
@caendesilva caendesilva merged commit efbb177 into new-asset-system Dec 8, 2024
6 checks passed
@caendesilva caendesilva deleted the improve-the-vite-integration branch December 8, 2024 17:24
@caendesilva caendesilva mentioned this pull request Dec 10, 2024
99 tasks
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.

2 participants