commaai / new-connect

a rewrite of comma connect

https://new-connect.connect-d5y.pages.dev/

MIT License

11 stars 29 forks source link

feat: added PWA asset generation #110

Open knownotunknown opened 3 weeks ago

knownotunknown commented 3 weeks ago

Features

Added and ran package.json script to handle PWA asset generation and integration
Used https://github.com/elegantapp/pwa-asset-generator for PWA asset generation

CI

Excluded PWA assets from bundle size calculation

PR 1 of 2 to close https://github.com/commaai/new-connect/issues/59.

2nd PR will include caching via service worker and testing via Lighthouse GitHub Actions. Could split up caching and testing if needed, but not sure if that's worth it/makes sense.

Original, full PR: https://github.com/commaai/new-connect/pull/108

github-actions[bot] commented 3 weeks ago

110.connect-d5y.pages.dev

Welcome to new-connect! Make sure to:

read the contributing guidelines
mark your PR as a draft until it's ready to review
post the preview on Discord; feedback from users will speedup the PR review

Mobile

Desktop

knownotunknown commented 2 weeks ago

Can we gitignore the generated assets and just generate them at build-time?

Sure. I went ahead and created separate build and build:prod commands since generating pwa-assets increases total build time from 1s to 5s, and you only need to generate pwa-assets once anyways.

adeebshihadeh commented 2 weeks ago

Sure. I went ahead and created separate build and build:prod commands since generating pwa-assets increases total build time from 1s to 5s, and you only need to generate pwa-assets once anyways.

This is not a good tradeoff. What happens if the assets get updated? How do I discover this build:prod command? It should just be part of the normal build. One case is 10x better than two cases.

knownotunknown commented 2 weeks ago

Sure. I went ahead and created separate build and build:prod commands since generating pwa-assets increases total build time from 1s to 5s, and you only need to generate pwa-assets once anyways.

This is not a good tradeoff. What happens if the assets get updated? How do I discover this build:prod command? It should just be part of the normal build. One case is 10x better than two cases.

That's fair. Fixed. I wasn't sure about the reasoning for .gitignoring the pwa-assets since those assets would probably rarely change anyways, and it added an extra build step, so I thought we were really just optimizing for cleanliness.

Was the main motivation of .gitignoring to keep our assets up-to-date with the source svg?

adeebshihadeh commented 2 weeks ago

Yes, this way they're never out of date and the SVG is the source of truth. Generally, you don't want to check in build products.