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

chore!: dist files optimizations (sw asset name changes) #97

Merged
merged 14 commits into from
Mar 11, 2024

Conversation

SgtPooki
Copy link
Member

@SgtPooki SgtPooki commented Mar 8, 2024

  • dont gzip html files

Title

Gzip compression for prod builds only.

Description

  • Moves compression plugin to prod config only.
  • Removes compression for webpack-dev-server (should speed up dev builds).
  • removes html files from compression

related to #96 & #93

On main (without #89 and #94)

Development builds are 20M
Prod builds are 3.5M

Notes & open questions

Notes

FYSI, my nginx config required the following changes:

  1. gzip_static on; to serve pre-compressed files
  2. building nginx with --with-http_gzip_static_module to enable serving pre-compressed files
    • brew tap denji/nginx
    • brew install nginx-full --with-http_gzip_static_module
  • For IPFS hosted sites, we likely need to add some entries:
    • /*.js /:splat.js.gz 301
    • /:splat.css /:splat.css.gz 301

Questions

  • does anyone see any issues with this approach?
  • are the splat entries above correct?

Change checklist

  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation if necessary (this includes comments as well)
  • I have added tests that prove my fix is effective or that my feature works

numiterations: 15,
minRatio: 0.8
},
deleteOriginalAssets: true
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

deleteOriginalAssets was in the wrong location (inside compressionOptions) previously.

@SgtPooki
Copy link
Member Author

SgtPooki commented Mar 9, 2024

ok.. for testing redirects locally, taken from https://github.com/ipfs/kubo/blob/e22f47ae4b4d83cf5e3f1c901c10905ebd69d492/test/sharness/t0109-gateway-web-_redirects.sh#L3


test_should_contain() {
  test "$#" = 2 || error "bug in the test script: not 2 parameters to test_should_contain"
  if ! grep -q "$1" "$2"
  then
    echo "'$2' does not contain '$1', it contains:"
    cat "$2"
    return 1
  fi
}

we can do the following with a locally running kubo node:

> npm run clean && npm run build && ls dist && CID=$(npx kubo add --cid-version 1 -Qr dist)
> curl -sD - "http://127:0.0.1:8081/ipfs/$CID/config" > response  &&
  test_should_contain "302 Moved Permanently" response

and it will fail if the redirect doesn't match.

@SgtPooki
Copy link
Member Author

SgtPooki commented Mar 9, 2024

Before merging from main (with all the updates from #89 and others)

image

After merging from main:

image

@SgtPooki
Copy link
Member Author

SgtPooki commented Mar 9, 2024

without node-polyfills-plugin (78a564f)

image

@SgtPooki
Copy link
Member Author

SgtPooki commented Mar 9, 2024

i've got it down a bit, and normalized some of the output chunk names in 64335ff (#97):

image

@SgtPooki SgtPooki changed the title chore: gzip compression only on prod builds chore: dist files optimizations Mar 11, 2024
Copy link
Member

@lidel lidel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.gz produced by CompressionPlugin are not used, we have gzip support on inbrowser.link without them. see details below

let's focus this PR to be about minimizing browser bundle, infra stuff is out of scope

public/_redirects Outdated Show resolved Hide resolved
webpack.config.js Outdated Show resolved Hide resolved
@SgtPooki
Copy link
Member Author

the service worker name is going to change with this:

On main branch

> npm run clean && npm i && npm run build && npx http-server dist

[2024-03-11T22:57:31.489Z]  "GET /" "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/122.0.0.0 Safari/537.36"
[2024-03-11T22:57:31.626Z]  "GET /vendor-react.js" "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/122.0.0.0 Safari/537.36"
[2024-03-11T22:57:31.639Z]  "GET /main.js" "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/122.0.0.0 Safari/537.36"
[2024-03-11T22:57:31.639Z]  "GET /sw.js" "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/122.0.0.0 Safari/537.36"
[2024-03-11T22:57:31.783Z]  "GET /favicon.ico" "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/122.0.0.0 Safari/537.36"
[2024-03-11T22:57:31.946Z]  "GET /favicon.ico" "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/122.0.0.0 Safari/537.36"
[2024-03-11T22:57:31.970Z]  "GET /519.js" "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/122.0.0.0 Safari/537.36"

Service worker source from chrome devtools

image

on this branch

> npm run clean && npm run build && npx http-server dist

http-server settings:
CORS: disabled
Cache: 3600 seconds
Connection Timeout: 120 seconds
Directory Listings: visible
AutoIndex: visible
Serve GZIP Files: false
Serve Brotli Files: false
Default File Extension: none

Available on:
  http://127.0.0.1:8080
  http://192.168.1.227:8080
Hit CTRL-C to stop the server

# Load the site in browser...


[2024-03-11T23:02:30.242Z]  "GET /519.js" "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/122.0.0.0 Safari/537.36"
(node:91351) [DEP0066] DeprecationWarning: OutgoingMessage.prototype._headers is deprecated
(Use `node --trace-deprecation ...` to show where the warning was created)
[2024-03-11T23:02:30.256Z]  "GET /519.js" Error (404): "Not found"
[2024-03-11T23:03:35.909Z]  "GET /" "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/122.0.0.0 Safari/537.36"
[2024-03-11T23:03:36.064Z]  "GET /vendor-react.js" "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/122.0.0.0 Safari/537.36"
[2024-03-11T23:03:36.070Z]  "GET /main.js" "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/122.0.0.0 Safari/537.36"
[2024-03-11T23:03:36.070Z]  "GET /vendor-rest.js" "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/122.0.0.0 Safari/537.36"
[2024-03-11T23:03:36.202Z]  "GET /favicon.ico" "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/122.0.0.0 Safari/537.36"
[2024-03-11T23:03:36.351Z]  "GET /favicon.ico" "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/122.0.0.0 Safari/537.36"
[2024-03-11T23:03:36.377Z]  "GET /sw.js" "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/122.0.0.0 Safari/537.36"
[2024-03-11T23:03:36.391Z]  "GET /vendor-rest.js" "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/122.0.0.0 Safari/537.36"
[2024-03-11T23:03:36.574Z]  "GET /sw.js" "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/122.0.0.0 Safari/537.36"

Service worker source from chrome devtools

image

Copy link
Member Author

@SgtPooki SgtPooki left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

self review

@@ -1,7 +1,7 @@
import { log } from './lib/logger.ts'

export async function registerServiceWorker (): Promise<ServiceWorkerRegistration> {
const swRegistration = await navigator.serviceWorker.register(new URL('sw.ts', import.meta.url))
const swRegistration = await navigator.serviceWorker.register(new URL(/* webpackChunkName: "sw" */'sw.ts', import.meta.url))
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

name is prefixed in webpack config

@@ -103,6 +105,21 @@ self.addEventListener('fetch', (event) => {
* Functions
******************************************************
*/
async function getVerifiedFetch (): Promise<VerifiedFetch> {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

moved in here because nothing else is using it

Comment on lines 14 to 20
chunks: 'async',
minSize: 20000,
minRemainingSize: 0,
minChunks: 1,
maxAsyncRequests: 30,
maxInitialRequests: 30,
enforceSizeThreshold: 1024,
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

most of these are defaults

Comment on lines +182 to +185
output: {
path: paths.build,
publicPath: '/',
filename: 'ipfs-sw-[name].js'
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dev and prod build produce the same assets now

@SgtPooki
Copy link
Member Author

final build output:

image image

note that we are no longer creating pre-compressed gzip assets, but webpack bundle analyzer gives us it's guess at what that size would be.

@SgtPooki SgtPooki changed the title chore: dist files optimizations chore!: dist files optimizations (sw asset name changes) Mar 11, 2024
@SgtPooki SgtPooki merged commit 0c81f2a into main Mar 11, 2024
19 checks passed
@SgtPooki SgtPooki deleted the feat/asset-optimization branch March 11, 2024 23:44
@SgtPooki SgtPooki mentioned this pull request Mar 12, 2024
2color added a commit that referenced this pull request Mar 12, 2024
* origin/main:
  feat: allow loading config page from subdomain after sw registration (#96)
  chore!: dist files optimizations (sw asset name changes) (#97)
  Update src/components/CidRenderer.tsx
  chore: remove in-page rendering
@lidel lidel mentioned this pull request Mar 15, 2024
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