- 
          
 - 
                Notifications
    
You must be signed in to change notification settings  - Fork 871
 
Add asset-map compilation if importmap is defined #863
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please note that according to the AssetMapper documentation, asset compilation should be performed in the production environment, otherwise Symfony will stop serving assets dynamically:
If you run the
asset-map:compilecommand on your development machine, you won't see any changes made to your assets when reloading the page. To resolve this, delete the contents of thepublic/assets/directory. This will allow your Symfony application to serve those assets dynamically again.
https://symfony.com/doc/7.3/frontend/asset_mapper.html#serving-assets-in-dev-vs-prod
It is also necessary to provide for the possibility of installing dependencies, as they may not be installed:
All packages in
importmap.phpare downloaded into anassets/vendor/directory, which should be ignored by git (the Flex recipe adds it to.gitignorefor you). You'll need to run the following command to download the files on other computers if some are missing:
php bin/console importmap:install
https://symfony.com/doc/7.3/frontend/asset_mapper.html#importing-3rd-party-javascript-packages
In addition, the current solution will run dependency installation and asset compilation every time the container is started, and even if various checks are added, this is not the best solution, since building the project at this stage takes time (the result is not cached) and can lead to various errors (e.g., due to network issues). That is why the installation of Composer dependencies for the production environment is performed during image build:
Lines 82 to 95 in 322b869
| COPY --link composer.* symfony.* ./ | |
| RUN set -eux; \ | |
| composer install --no-cache --prefer-dist --no-dev --no-autoloader --no-scripts --no-progress | |
| # copy sources | |
| COPY --link . ./ | |
| RUN rm -Rf frankenphp/ | |
| RUN set -eux; \ | |
| mkdir -p var/cache var/log; \ | |
| composer dump-autoload --classmap-authoritative --no-dev; \ | |
| composer dump-env prod; \ | |
| composer run-script --no-dev post-install-cmd; \ | |
| chmod +x bin/console; sync; | 
          
 I added a check on APP_ENV so that this is only run in prod mode, you are right :) As for using this in build phase instead of run phase, I tried adding this to the dockerfile in the frankenphp_prod build phase, at the very end of the file: This works, and seems better placed then ?  | 
    
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should use the set -eux command to enable tracing and stop script execution in case of an error, as well as sync to synchronize file system changes. Additionally, you may want to combine all commands into a single layer.
a3dc3f6    to
    1f70c37      
    Compare
  
    This is usefull for plug-and-play production mode for asset-mapper
1f70c37    to
    9d3c1eb      
    Compare
  
    | 
           The path to  Does it make sense to replace the check for the existents of the   | 
    
| 
           Launching a full command would be slower, another way would be to check composer.json graph ?     if composer show symfony/asset-mapper >/dev/null 2>&1; then \
        php bin/console asset-map:compile; \
    fi; \Or even simpler with a grep ?     if grep -q '"symfony/asset-mapper"' composer.json; then \
        php bin/console asset-map:compile; \
    fi; \To be honest, I'm not sure a template should handle the weird use case where importmap is elsewhere. Looking at the conditions in the docker-entrypoint.sh, 	 Same for migrations:   So if you stray from the default config you are already accepting that this template's defaults might not work out  | 
    
This is useful for plug-and-play production mode for asset-mapper