Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .github/workflows/buildAdHoc.yml
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,7 @@ jobs:
BUCKET="s3://ad-hoc-expensify-cash/web/${{ inputs.APP_PR_NUMBER }}"

aws s3 sync dist "$BUCKET" --delete --acl public-read \
--exclude "*.br" \
--exclude "index.html" \
--exclude "service-worker.js" \
--exclude "workbox-*.js" \
Expand Down
11 changes: 10 additions & 1 deletion .github/workflows/deploy.yml
Original file line number Diff line number Diff line change
Expand Up @@ -741,7 +741,16 @@ jobs:

- name: Deploy to S3
run: |
aws s3 cp --recursive --acl public-read "$GITHUB_WORKSPACE"/dist ${{ env.S3_BUCKET }}/
aws s3 cp --recursive --acl public-read --exclude "*.br" "$GITHUB_WORKSPACE"/dist ${{ env.S3_BUCKET }}/

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

NAB: /docs/* can silently return the NewDot app shell.

The dist here combines the web build (which has .br twins) with the Storybook build under ./dist/docs (which does not).

Because the CDN rewrite currently applies to /docs/*, a request like:

/docs/sb-manager/runtime.js → /docs/sb-manager/runtime.js.br

can miss the file and still return 200 HTML because of the SPA fallback. This can cause ERR_CONTENT_DECODING_FAILED or the browser to reject the script due to the text/html MIME type. A 404-based check won't catch it.

Could we please:

  1. Exclude /docs/* from the CloudFront rewrite and add a comment explaining why.
  2. Update the comment in BrotliCompressionPlugin.ts:17 — a missing twin does not necessarily mean a 404 because of the SPA fallback.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Exclude /docs/* from the CloudFront rewrite and add a comment explaining why.

cc: @justinpersaud

I think we need this since /docs is the Storybook build, which doesn't go through rspack, so none of its assets have .br twins, but they land in the same bucket behind the same distribution. WDYT?

for PAIR in js:text/javascript css:text/css html:text/html svg:image/svg+xml wasm:application/wasm ttf:font/ttf; do
EXT="${PAIR%%:*}"
CONTENT_TYPE="${PAIR#*:}"
aws s3 cp --recursive --acl public-read --exclude "*" --include "*.${EXT}.br" \
--content-encoding br --content-type "$CONTENT_TYPE" \
"$GITHUB_WORKSPACE"/dist ${{ env.S3_BUCKET }}/
done
aws s3 cp --acl public-read --content-type 'application/json' --metadata-directive REPLACE ${{ env.S3_BUCKET }}/.well-known/apple-app-site-association ${{ env.S3_BUCKET }}/.well-known/apple-app-site-association
aws s3 cp --acl public-read --content-type 'application/json' --metadata-directive REPLACE ${{ env.S3_BUCKET }}/.well-known/apple-app-site-association ${{ env.S3_BUCKET }}/apple-app-site-association
env:
Expand Down
55 changes: 55 additions & 0 deletions config/rsbuild/BrotliCompressionPlugin.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
import type {Compiler} from '@rspack/core';

import {promisify} from 'util';
import zlib from 'zlib';

const PLUGIN_NAME = 'BrotliCompressionPlugin';

const brotliCompress = promisify(zlib.brotliCompress);

type Options = {
/** Every emitted asset whose name matches gets a `.br` twin. */
test: RegExp;
};

/**
* Rspack plugin that writes a Brotli 11 twin (`foo.js` -> `foo.js.br`) beside every emitted asset matching `test`.
* Every match gets one, however small or incompressible: the CDN rewrite appends `.br` blindly, and a missing twin doesn't even 404 — the SPA fallback answers 200
* with the app shell, so the browser either fails to decode that HTML as Brotli or refuses to run it as a script.
*/
class BrotliCompressionPlugin {
private readonly options: Options;

constructor(options: Options) {
this.options = options;
}

apply(compiler: Compiler): void {
const {Compilation, sources} = compiler.rspack;

// `thisCompilation` skips child compilations, whose assets end up in the parent's anyway.
compiler.hooks.thisCompilation.tap(PLUGIN_NAME, (compilation) => {
compilation.hooks.processAssets.tapPromise(
// OPTIMIZE_TRANSFER runs after minification, content hashing, HTML and the service worker are emitted,
// so the twins are made from the final bytes that will be deployed.
{name: PLUGIN_NAME, stage: Compilation.PROCESS_ASSETS_STAGE_OPTIMIZE_TRANSFER},
async () => {
const assets = compilation.getAssets().filter(({name, info}) => !info.compressed && this.options.test.test(name));

// zlib runs each compression on the libuv thread pool, so these proceed in parallel.
await Promise.all(
assets.map(async ({name, source, info}) => {
const compressed = await brotliCompress(source.buffer(), {params: {[zlib.constants.BROTLI_PARAM_QUALITY]: zlib.constants.BROTLI_MAX_QUALITY}});

// `compressed` is the webpack convention that tells other plugins (and this one on a rebuild) not to
// compress the twin again. A twin of a content-hashed file is as immutable as its original.
compilation.emitAsset(`${name}.br`, new sources.RawSource(compressed), {compressed: true, immutable: info.immutable});
}),
);
},
);
});
}
}

export default BrotliCompressionPlugin;
13 changes: 13 additions & 0 deletions config/rsbuild/rsbuild.common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ import SENTRY_APPLICATION_KEY from '../../src/libs/telemetry/sentryApplicationKe
import getAppVersion from '../../src/libs/VersionUtils.ts'; // eslint-disable-line @dword-design/import-alias/prefer-alias
import oxcReactCompilerConfig from '../babel/oxcReactCompilerConfig.js';
// @ts-expect-error -- Can't use .ts extensions without allowImportingTsExtensions in tsconfig
import BrotliCompressionPlugin from './BrotliCompressionPlugin.ts';
// @ts-expect-error -- Can't use .ts extensions without allowImportingTsExtensions in tsconfig
import CustomVersionFilePlugin from './CustomVersionFilePlugin.ts';
// @ts-expect-error -- Can't use .ts extensions without allowImportingTsExtensions in tsconfig
import ModuleInitTimingPlugin from './ModuleInitTimingPlugin.ts';
Expand Down Expand Up @@ -327,6 +329,7 @@ const getSharedConfiguration = ({file = '.env', isDevServer = false}: Environmen
*/
const getCommonConfiguration = async ({file = '.env', platform = 'web', isDevServer = false}: Environment): Promise<RsbuildConfig> => {
const isDevelopment = file === '.env' || file === '.env.development';
const shouldCompressWithBrotli = !isDevelopment && file !== '.env.adhoc';
const shared = getSharedConfiguration({file, platform, isDevServer});
const sharedRspackTool = shared.tools?.rspack;
const sentryWebpackPlugin = isDevelopment ? undefined : (await import('@sentry/webpack-plugin')).sentryWebpackPlugin;
Expand Down Expand Up @@ -419,6 +422,9 @@ const getCommonConfiguration = async ({file = '.env', platform = 'web', isDevSer
},
},
performance: {
// Rsbuild's default exclusion, plus the `.br` twins BrotliCompressionPlugin emits below: listing them would
// double the report with a meaningless "gzipped size" of already-Brotli-compressed bytes.
printFileSize: {exclude: (asset) => /\.(?:map|LICENSE\.txt|d\.(?:ts|mts|cts)|br)$/.test(asset.name)},
// We have to load the whole lottie player to get the player to work in offline mode
// heic-to library is used sparsely so we load it as a separate chunk to reduce initial bundle size
// ExpensifyIcons/illustrations chunks are loaded eagerly for offline support
Expand Down Expand Up @@ -502,6 +508,10 @@ const getCommonConfiguration = async ({file = '.env', platform = 'web', isDevSer
// all critical for offline boot, so we precache the lot. Everything in the
// App build is content-hashed, so growth here only costs first-install bytes.
maximumFileSizeToCacheInBytes: 10 * 1024 * 1024,
// Workbox's defaults, plus the `.br` twins BrotliCompressionPlugin emits: the service
// worker requests the original URLs and the CDN transparently serves the Brotli copy,
// so adding the twins to the precache as well would download every chunk twice.
exclude: [/\.map$/, /^manifest.*\.js$/, /\.br$/],
// Single-page app: any unmatched navigation should serve the cached app shell.
navigateFallback: '/index.html',
// Don't fall back for asset-like or .well-known requests.
Expand Down Expand Up @@ -591,6 +601,9 @@ const getCommonConfiguration = async ({file = '.env', platform = 'web', isDevSer
: []),
// This allows us to interactively inspect JS bundle contents, loader/plugin timings, and duplicate packages
...(process.env.ANALYZE_BUNDLE === 'true' ? [new RsdoctorRspackPlugin()] : []),
// Writes a Brotli 11 twin (`foo.js` -> `foo.js.br`) beside every deployable text/bytecode asset, so the CDN
// can serve it instead of compressing with gzip on the fly: 25-30% fewer bytes over the wire.
...(shouldCompressWithBrotli ? [new BrotliCompressionPlugin({test: /\.(?:js|css|html|svg|wasm|ttf)$/})] : []),
);

return afterShared;
Expand Down
Loading