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] No longer copy files to site output from the asset bundler command #2011

Merged

Conversation

caendesilva
Copy link
Member

@caendesilva caendesilva commented Nov 11, 2024

Abstract

This pull request changes where compiled assets are saved during build. Currently, they go to _site/media. This request moves them to _media to better match project structure and simplify the build process as this is unexpected and should just be handled by the build script

Motivation

Based on the documentation and project structure, I believe npm run build should save assets to _media directory. Here's my reasoning:

Our documented conventions establish that:

  • resources/assets contains source files that need compilation
  • _media contains compiled/minified files
  • _site/media is the final output directory for serving to users

This pattern makes sense because:

  • It separates source, compiled, and output files clearly
  • _media acts as a version-controlled cache of compiled assets
  • When running php hyde build, it can simply copy from _media to _site/media
  • Other developers can use the pre-compiled assets in _media without needing to run npm/node
  • The _site directory is typically gitignored and meant for final output only

So while the current Vite config outputs to _site/media and then copies back to _media, I believe the more logical flow would be:

  1. Vite builds assets from resources/assets into _media
  2. php hyde build copies from _media to _site/media

This maintains a cleaner separation of concerns and better matches the documented purpose of each directory.

@caendesilva caendesilva force-pushed the no-longer-copy-assets-from-asset-bundler branch from c256ece to 0e81adb Compare November 11, 2024 13:04
@caendesilva caendesilva merged commit ff21c4a into new-asset-system Nov 11, 2024
10 checks passed
@caendesilva caendesilva deleted the no-longer-copy-assets-from-asset-bundler branch November 11, 2024 13:06
Copy link

codecov bot commented Nov 11, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (a1b0783) to head (0e81adb).
Report is 2 commits behind head on new-asset-system.

Additional details and impacted files
@@                 Coverage Diff                  @@
##             new-asset-system     #2011   +/-   ##
====================================================
  Coverage              100.00%   100.00%           
  Complexity               1891      1891           
====================================================
  Files                     194       194           
  Lines                    5044      5044           
====================================================
  Hits                     5044      5044           

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

@caendesilva caendesilva changed the title No longer copy files to site output from the asset bundler command [2.x] No longer copy files to site output from the asset bundler command Nov 11, 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.

1 participant