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

Add support to override manifest_path as a configuration #195

Merged
merged 1 commit into from
Sep 4, 2024

Conversation

fabiormoura
Copy link
Contributor

@fabiormoura fabiormoura commented Jun 27, 2024

What is the problem this is solving?

The current implementation has a hardcoded manifest path making it difficult to adapt to various deployment scenarios.

In my scenario, a Docker image needs to be be built for the application to be deployed. The build process works as follows:

  • The public/assets directory is cached across builds by default.
  • public/assets is mounted at the beginning of the image build process.
  • rails assets:precompile is executed.
  • Other steps are executed that are not pertinent.
  • Files in public/assets are uploaded to a cloud bucket.
  • public/assets is unmounted.
  • The directory is no longer available.
  • The docker image is pushed to the registry.

When the docker container is started in a production environment, the public/assets folder does not exist in the local path as all the files are served through the CDN. This causes Propshaft to use the Dynamic resolver, which needs to compute all asset digests at runtime.

The code changes prevents Propshaft from using the Dynamic resolver and allows the Static resolver to be used even if public/assets is absent in the local path which is ideal for setups where assets are served via CDN.

@fabiormoura fabiormoura force-pushed the manifest_path_configuration branch from 41da5b1 to be1deec Compare June 27, 2024 15:03
@fabiormoura fabiormoura marked this pull request as ready for review June 27, 2024 15:26
@rafaelfranca rafaelfranca merged commit f06b3f9 into rails:main Sep 4, 2024
3 checks passed
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