Skip to content

Upgrade react-fast-pdf and webpack #80160

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.

Sign up for GitHub

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

Merged
roryabraham merged 11 commits into from
Apr 6, 2026
Merged

Upgrade react-fast-pdf and webpack #80160

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
d9564d2
Upgrade `react-fast-pdf` and `webpack`
andriivitiv Jan 21, 2026
7b9232e
Merge branch 'main' of github.com:Expensify/App into 76303-upgrade-re…
andriivitiv Feb 10, 2026
2a32a51
Merge branch 'main' of github.com:Expensify/App into 76303-upgrade-re…
andriivitiv Mar 3, 2026
e031ca1
Upgrade `pdfjs-dist` and remove patch
andriivitiv Mar 3, 2026
4342062
Merge branch 'main' of github.com:Expensify/App into 76303-upgrade-re…
andriivitiv Mar 25, 2026
a178522
Switch to legacy build of pdf.js
andriivitiv Mar 25, 2026
be31af9
Merge branch 'main' of github.com:Expensify/App into 76303-upgrade-re…
andriivitiv Apr 3, 2026
8001a2c
Add polyfills instead of using legacy PDF worker
andriivitiv Apr 3, 2026
196040b
Add polyfill for Uint8Array.toHex
andriivitiv Apr 3, 2026
6996e7d
Revert "Add polyfill for Uint8Array.toHex"
andriivitiv Apr 6, 2026
0064ae7
Revert "Add polyfills instead of using legacy PDF worker"
andriivitiv Apr 6, 2026
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
8 changes: 5 additions & 3 deletions config/webpack/webpack.common.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,8 +162,8 @@ const getCommonConfiguration = ({file = '.env', platform = 'web'}: Environment):
{from: 'assets/fonts/web', to: 'fonts'},
{from: 'assets/sounds', to: 'sounds'},
{from: 'assets/pdfs', to: 'pdfs'},
{from: 'node_modules/react-pdf/dist/esm/Page/AnnotationLayer.css', to: 'css/AnnotationLayer.css'},
{from: 'node_modules/react-pdf/dist/esm/Page/TextLayer.css', to: 'css/TextLayer.css'},
{from: 'node_modules/react-pdf/dist/Page/AnnotationLayer.css', to: 'css/AnnotationLayer.css'},
{from: 'node_modules/react-pdf/dist/Page/TextLayer.css', to: 'css/TextLayer.css'},
{from: '.well-known/apple-app-site-association', to: '.well-known/apple-app-site-association', toType: 'file'},
{from: '.well-known/assetlinks.json', to: '.well-known/assetlinks.json'},

Expand Down Expand Up @@ -260,7 +260,7 @@ const getCommonConfiguration = ({file = '.env', platform = 'web'}: Environment):
// We are importing this worker as a string by using asset/source otherwise it will default to loading via an HTTPS request later.
// This causes issues if we have gone offline before the pdfjs web worker is set up as we won't be able to load it from the server.
{
test: new RegExp('node_modules/pdfjs-dist/build/pdf.worker.min.mjs'),
test: new RegExp('node_modules/pdfjs-dist/legacy/build/pdf.worker.min.mjs'),
type: 'asset/source',
},

Expand Down Expand Up @@ -330,6 +330,8 @@ const getCommonConfiguration = ({file = '.env', platform = 'web'}: Environment):
'victory-native': path.resolve(dirname, '../../node_modules/victory-native/src/index.ts'),
// Required for @shopify/react-native-skia web support
'react-native/Libraries/Image/AssetRegistry': false,
// Use legacy build of pdfjs-dist to support older browsers
Copy link
Copy Markdown
Contributor

@roryabraham roryabraham Mar 30, 2026

Choose a reason for hiding this comment

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

Where was this discussed? Which older browsers specificallly are we talking about? We can check production data to see if it matters for us.

Copy link
Copy Markdown
Contributor

@QichenZhu QichenZhu Mar 31, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

It was discussed here. The new features are supported from Chrome 145, released last month.

  1. getOrInsert

    Screenshot 2026-03-31 at 1 02 22 PM

  2. getOrInsertComputed

    Screenshot 2026-03-31 at 1 01 45 PM

Copy link
Copy Markdown
Contributor

@roryabraham roryabraham Mar 31, 2026

Choose a reason for hiding this comment

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

would an alternate solution be to polyfill these browser APIs?

Copy link
Copy Markdown
Contributor

@QichenZhu QichenZhu Apr 1, 2026

Choose a reason for hiding this comment

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

Yeah it's polyfillable: https://github.com/tc39/proposal-upsert?tab=readme-ov-file#polyfill.

@andriivitiv what do you think?

Copy link
Copy Markdown
Contributor Author

@andriivitiv andriivitiv Apr 2, 2026

Choose a reason for hiding this comment

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

It is possible for normal compile-time imports, but not for the worker because it is being bundled as a text file (type: 'asset/source'), which bypasses Babel and its polyfills. And changing this will cause issues while offline

App/config/webpack/webpack.common.ts

Lines 260 to 265 in 59fe031

// We are importing this worker as a string by using asset/source otherwise it will default to loading via an HTTPS request later.
// This causes issues if we have gone offline before the pdfjs web worker is set up as we won't be able to load it from the server.
{
test: new RegExp('node_modules/pdfjs-dist/build/pdf.worker.min.mjs'),
type: 'asset/source',
},

Copy link
Copy Markdown
Contributor

@QichenZhu QichenZhu Apr 3, 2026

Choose a reason for hiding this comment

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

@andriivitiv is it possible to put polyfills in src/polyfills instead of using babel?

Copy link
Copy Markdown
Contributor

@roryabraham roryabraham Apr 3, 2026

Choose a reason for hiding this comment

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

claude seems to think we can make the polyfill work without using a legacy version of pdfjs:

diff 
diff --git a/Mobile-Expensify b/Mobile-Expensify
index 15c7bec2369..f008b412836 160000
--- a/Mobile-Expensify
+++ b/Mobile-Expensify
@@ -1 +1 @@
-Subproject commit 15c7bec2369402e7e391d56a450fa6b3c97496d2
+Subproject commit f008b412836b7b26e38715cb148bcc34dd11d6df
diff --git a/config/webpack/webpack.common.ts b/config/webpack/webpack.common.ts
index e4c61a56502..00ea07212fd 100644
--- a/config/webpack/webpack.common.ts
+++ b/config/webpack/webpack.common.ts
@@ -246,6 +246,8 @@ const getCommonConfiguration = ({file = '.env', platform = 'web'}: Environment):
                 {
                     test: /\.(js|ts)x?$/,
                     loader: 'babel-loader',
+                    // asset/source imports (?raw) must not be transformed so the emitted string matches disk (e.g. pdf worker polyfill)
+                    resourceQuery: {not: [/raw/]},
 
                     /**
                      * Exclude node_modules except any packages we need to convert for rendering HTML because they import
@@ -260,7 +262,7 @@ const getCommonConfiguration = ({file = '.env', platform = 'web'}: Environment):
                 // We are importing this worker as a string by using asset/source otherwise it will default to loading via an HTTPS request later.
                 // This causes issues if we have gone offline before the pdfjs web worker is set up as we won't be able to load it from the server.
                 {
-                    test: new RegExp('node_modules/pdfjs-dist/legacy/build/pdf.worker.min.mjs'),
+                    test: new RegExp('node_modules/pdfjs-dist/build/pdf.worker.min.mjs'),
                     type: 'asset/source',
                 },
 
@@ -330,8 +332,8 @@ const getCommonConfiguration = ({file = '.env', platform = 'web'}: Environment):
                 'victory-native': path.resolve(dirname, '../../node_modules/victory-native/src/index.ts'),
                 // Required for @shopify/react-native-skia web support
                 'react-native/Libraries/Image/AssetRegistry': false,
-                // Use legacy build of pdfjs-dist to support older browsers
-                'pdfjs-dist$': path.resolve(dirname, '../../node_modules/pdfjs-dist/legacy/build/pdf.mjs'),
+                // Main-thread pdf.js (worker uses standard build + map upsert polyfill; see src/polyfills/mapUpsertPolyfill.js)
+                'pdfjs-dist$': path.resolve(dirname, '../../node_modules/pdfjs-dist/build/pdf.mjs'),
                 // Module alias for web
                 // https://webpack.js.org/configuration/resolve/#resolvealias
                 '@assets': path.resolve(dirname, '../../assets'),
diff --git a/src/components/PDFThumbnail/index.tsx b/src/components/PDFThumbnail/index.tsx
index 2f41ee9d0ed..bc751089bd1 100644
--- a/src/components/PDFThumbnail/index.tsx
+++ b/src/components/PDFThumbnail/index.tsx
@@ -1,6 +1,8 @@
 import 'core-js/proposals/promise-with-resolvers';
+import '@src/polyfills/mapUpsertPolyfill';
+import mapUpsertPolyfillSource from '@src/polyfills/mapUpsertPolyfill?raw';
 // eslint-disable-next-line import/extensions
-import pdfWorkerSource from 'pdfjs-dist/legacy/build/pdf.worker.min.mjs';
+import pdfWorkerSource from 'pdfjs-dist/build/pdf.worker.min.mjs';
 import React, {useMemo, useState} from 'react';
 import {View} from 'react-native';
 import {Document, pdfjs, Thumbnail} from 'react-pdf';
@@ -9,7 +11,7 @@ import useThemeStyles from '@hooks/useThemeStyles';
 import PDFThumbnailError from './PDFThumbnailError';
 import type PDFThumbnailProps from './types';
 
-pdfjs.GlobalWorkerOptions.workerSrc = URL.createObjectURL(new Blob([pdfWorkerSource], {type: 'text/javascript'}));
+pdfjs.GlobalWorkerOptions.workerSrc = URL.createObjectURL(new Blob([`${mapUpsertPolyfillSource}\n${pdfWorkerSource}`], {type: 'text/javascript'}));
 
 function PDFThumbnail({previewSourceURL, style, enabled = true, onPassword, onLoadError, onLoadSuccess}: PDFThumbnailProps) {
     const styles = useThemeStyles();
diff --git a/src/types/global.d.ts b/src/types/global.d.ts
index 76fe292e8a8..4918672ed22 100644
--- a/src/types/global.d.ts
+++ b/src/types/global.d.ts
@@ -27,6 +27,11 @@ declare module '*.lottie' {
     export default value;
 }
 
+declare module '*?raw' {
+    const content: string;
+    export default content;
+}
+
 // eslint-disable-next-line @typescript-eslint/consistent-type-definitions
 interface Window {
     setSupportToken: (token: string, email: string, accountID: number) => void;
diff --git a/src/types/modules/pdf.worker.d.ts b/src/types/modules/pdf.worker.d.ts
index a6d70e529b7..c636372b411 100644
--- a/src/types/modules/pdf.worker.d.ts
+++ b/src/types/modules/pdf.worker.d.ts
@@ -1 +1 @@
-declare module 'pdfjs-dist/legacy/build/pdf.worker.min.mjs';
+declare module 'pdfjs-dist/build/pdf.worker.min.mjs';

Copy link
Copy Markdown
Contributor Author

@andriivitiv andriivitiv Apr 3, 2026

Choose a reason for hiding this comment

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

Updated as suggested.

Tested this on

  • Safari 18.6: there was an error in the console. Added src/polyfills/ReadableStream.js to fix it.
  • Chrome 133: PDFs fail to open due to a missing polyfill for Uint8Array.prototype.toHex() (added in Chrome 140). Should we add a polyfill for this, or are versions 139 and older considered too old?

Copy link
Copy Markdown
Contributor

@roryabraham roryabraham Apr 3, 2026

Choose a reason for hiding this comment

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

Let’s add the polyfill. I think generally we want to support major browsers going back 2-3 years

'pdfjs-dist$': path.resolve(dirname, '../../node_modules/pdfjs-dist/legacy/build/pdf.mjs'),
// Module alias for web
// https://webpack.js.org/configuration/resolve/#resolvealias
'@assets': path.resolve(dirname, '../../assets'),
Expand Down
Loading
Loading