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

Avoid or document the need to run make build when modifying static assets #428

Open
robertknight opened this issue Mar 31, 2021 · 2 comments

Comments

@robertknight
Copy link
Member

robertknight commented Mar 31, 2021

Currently make build needs to be run locally when making changes to static assets and the minified assets are placed in the source tree rather than a separate build directory. This is not immediately obvious or documented in the README. It is also surprising coming from our other apps where minified assets are kept separate from the source tree.

My preference here would be to not minify assets at all for local development and only minify as part of the Docker image build, as we do for our other apps. A more minimal change is to make sure the steps to update static assets are documented both in the README and the description output when running make with no arguments.

@robertknight
Copy link
Member Author

I encountered this again today while trying to update PDF.js to the latest release and being confused as to why changes to the pdfjs-init.js script did not take effect.

Looking at the assets that we have, I think the only processing for static assets that is worthwhile and can't be left to the CDN (eg. Cloudflare can handle gzip compression) is minifying the PDF.js build, since that's the only large asset where compression is worthwhile.

For developer familiarity we might want to consider making the static asset setup more alike our other projects:

  • Place generated assets in the build/ directory at the root of the project
  • Use a gulpfile to handle populating the build/ directory
  • Use h-assets to serve assets from the build/ directory

@robertknight
Copy link
Member Author

Static asset serving based on h-assets was added in #905 for a new frontend app within Via (the video player). We still have the existing system for serving the PDF viewer and I'd like to unify the two in future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant