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

[Do Not Merge: Spike] => feat(performance): Add basic service-worker setup #14771

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

damassi
Copy link
Member

@damassi damassi commented Nov 2, 2024

⚠️ Do not merge: Spike for discussion!

The type of this PR is: Feat

This PR solves DIA-960

Description

This adds service worker ("Progressive Web App") support to Force via Google's Workbox plugin.

Service workers are specialized JavaScript assets that act as proxies between web browsers and web servers. They aim to improve reliability by providing offline access, as well as boost page performance.

Some various docs:

Comparisons:

⚠️ Reviewers! If you pull this down and test, be sure to go to devtools > application > web-workers and hit 'unregister worker' after you're done testing the PR. The worker seems to persist per domain, so checking out a new branch and making code changes in dev seems to create issues if the worker is still registered in the background.

This is a big subject and there's room to debate whether this is truly valuable with the defaults out of the box, vs normal http caching. However, service workers are very powerful and feature-rich, and provide complete control over various things. So this wires up the functionality, and we can explore how this might benefit us.

They also support robust offline access -- that's what service-workers are typically used for -- with js assets that are pre-cached via webpack, so pages don't hard-error out at the asset level, but we still need to perform fetch for data. So again, questionable if this provides much value in Artsy's use-case. (Something like this would be very cool for Eigen tho -- if it were a web-app.)

Network responses can also be pushed into the cache. Here's a rough example:

{
  urlPattern: /^https:\/\/metaphysics-cdn\.artsy\.net\/.*$/,
  handler: 'NetworkFirst', // Or use 'StaleWhileRevalidate' 
  options: {
    cacheName: 'api-cache',
    networkTimeoutSeconds: 10, 
    expiration: {
      maxEntries: 50, 
      maxAgeSeconds: 5 * 60, // Cache for 5 minutes
    },
    cacheableResponse: { statuses: [0, 200] }, 
  },
}

I spiked on this many many years ago, but back then force was much more fragmented. Now that its unified, and we're on a performance kick, wanted to revisit and implement this as a spike, which is much easier due to site-wide consistency and Google's Webpack plugin.

@joeyAghion and @mzikherman - perhaps y'all can train your sharp thinking on this to see if there might be some yield? We can go deep here.

Some Screen-caps:

Screenshot 2024-11-02 at 12 48 33 PM

Example of first load, vs subsequent load:

service-worker.mp4

Example of offline not erroring out:

offline.mp4

cc @artsy/diamond-devs

@damassi damassi self-assigned this Nov 2, 2024
@artsy-peril
Copy link
Contributor

artsy-peril bot commented Nov 2, 2024

workbox-webpack-plugin

Author: Google's Web DevRel Team and Google's Aurora Team

Description: A plugin for your Webpack build process, helping you generate a manifest of local files that workbox-sw should precache.

Homepage: https://github.com/GoogleChrome/workbox

Createdover 7 years ago
Last Updatedabout 5 hours ago
LicenseMIT
Maintainers8
Releases101
Direct Dependenciesupath, pretty-bytes, workbox-build, webpack-sources and fast-json-stable-stringify
Keywordsworkbox, workboxjs, webpack, service worker, caching, fetch requests, offline and file manifest
README

This module's documentation can be found at https://developer.chrome.com/docs/workbox/modules/workbox-webpack-plugin/

New dependencies added: workbox-webpack-plugin.

Generated by 🚫 dangerJS against cb273a1

@damassi damassi requested a review from a team November 2, 2024 20:17
Copy link

relativeci bot commented Nov 4, 2024

#820 Bundle Size — 9.57MiB (+0.26%).

cb273a1(current) vs ffe4f84 main#484(baseline)

Important

Bundle introduced 1 and removed 4 duplicate packages – View changed duplicate packages

Warning

Bundle introduced 3 new packages: web-vitals, @sentry-internal/browser-utils, stylis – View changed packages

Bundle metrics  Change 9 changes Regression 1 regression Improvement 3 improvements
                 Current
#820
     Baseline
#484
Improvement  Initial JS 3.84MiB(-2.79%) 3.95MiB
No change  Initial CSS 0B 0B
Change  Cache Invalidation 82.37% 2.04%
Change  Chunks 139(-2.8%) 143
Change  Assets 144(-1.37%) 146
Change  Modules 5666(+0.51%) 5637
Regression  Duplicate Modules 472(+3.74%) 455
Change  Duplicate Code 6.17%(+4.93%) 5.88%
Improvement  Packages 282(-3.09%) 291
Improvement  Duplicate Packages 39(-7.14%) 42
Bundle size by type  Change 2 changes Regression 1 regression Improvement 1 improvement
                 Current
#820
     Baseline
#484
Regression  JS 9.35MiB (+0.35%) 9.31MiB
Improvement  Other 230.18KiB (-3.22%) 237.84KiB

Bundle analysis reportBranch damassi/service-workersProject dashboard


Generated by RelativeCIDocumentationReport issue

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

Successfully merging this pull request may close these issues.

1 participant