Skip to content

Conversation

@emmadesilva
Copy link
Member

@emmadesilva emmadesilva 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.

@emmadesilva emmadesilva force-pushed the no-longer-copy-assets-from-asset-bundler branch from c256ece to 0e81adb Compare November 11, 2024 13:04
@emmadesilva emmadesilva merged commit ff21c4a into new-asset-system Nov 11, 2024
10 checks passed
@emmadesilva emmadesilva deleted the no-longer-copy-assets-from-asset-bundler branch November 11, 2024 13:06
@codecov
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.

@emmadesilva emmadesilva 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.

2 participants