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

Migrate to CSS modules / Support NextJS #284

Merged
merged 3 commits into from
Sep 9, 2024

Conversation

0kzh
Copy link
Contributor

@0kzh 0kzh commented Sep 6, 2024

Closes #156

Based on the work in
#62 (comment)
https://github.com/teesis/react-pdf-highlighter/tree/next-compatibility

Tests
Screenshot 2024-09-05 at 6 37 59 PM

Notes

  • Switches to using Vite build, since we need to export the main js/ts files, as well as two .css files (one for css modules, one for global css)
  • Would break current usage, new users would need to
import "react-pdf-highlighter/dist/index.css";

@0kzh 0kzh changed the title Migrate to CSS modules Migrate to CSS modules / Support NextJS Sep 6, 2024
@0kzh 0kzh force-pushed the convert-to-css-modules branch from 10049cf to 4c0bdbd Compare September 6, 2024 03:01
@0kzh 0kzh force-pushed the convert-to-css-modules branch from c700bca to 800d775 Compare September 6, 2024 03:31
@agentcooper
Copy link
Owner

Hey @0kzh, thanks a lot for this!

I made a branch with some small changes on top of your PR – 3d07e2e, could you please check if they make sense for you? The output still has separate a CSS file, but the JS modules are preserved (instead of one big minified file).

@0kzh
Copy link
Contributor Author

0kzh commented Sep 7, 2024

Hey @0kzh, thanks a lot for this!

I made a branch with some small changes on top of your PR – 3d07e2e, could you please check if they make sense for you? The output still has separate a CSS file, but the JS modules are preserved (instead of one big minified file).

Tried it, think it works for the most part! A couple notes though:

Trying to import types like this fails, probably due to ESM module resolution? (this was broken in my fork too)

Screenshot 2024-09-07 at 11 37 03 AM

I'm not sure if export * from "./types" works, but explicitly exporting the named exports works:

export type { LTWH, LTWHP, Scaled, Position, ScaledPosition, Content, HighlightContent, Comment, HighlightComment, NewHighlight, IHighlight, ViewportHighlight, Viewport, Page } from "./types";

I made these changes to try and get the library to play well with Turbopack but the underlying pdf-js lib had a couple of patches I needed to make:

diff --git a/node_modules/pdfjs-dist/build/pdf.mjs b/node_modules/pdfjs-dist/build/pdf.mjs
index 7906d1a..4875738 100644
--- a/node_modules/pdfjs-dist/build/pdf.mjs
+++ b/node_modules/pdfjs-dist/build/pdf.mjs
@@ -11878,8 +11878,17 @@ class PDFWorker {
       if (this.#mainThreadWorkerMessageHandler) {
         return this.#mainThreadWorkerMessageHandler;
       }
-      const worker = await import( /*webpackIgnore: true*/this.workerSrc);
-      return worker.WorkerMessageHandler;
+      // Dynamically create a script element to load the worker script
+      return new Promise((resolve, reject) => {
+        const script = document.createElement('script');
+        script.src = this.workerSrc ?? "https://unpkg.com/[email protected]/build/pdf.worker.min.mjs";
+        script.type = 'module';
+        script.onload = () => {
+          resolve(globalThis.pdfjsWorker.WorkerMessageHandler);
+        };
+        script.onerror = reject;
+        document.head.appendChild(script);
+      });
     };
     return shadow(this, "_setupFakeWorkerGlobal", loader());
   }
diff --git a/node_modules/pdfjs-dist/legacy/web/pdf_viewer.mjs b/node_modules/pdfjs-dist/legacy/web/pdf_viewer.mjs
index fad5698..ce02cd1 100644
--- a/node_modules/pdfjs-dist/legacy/web/pdf_viewer.mjs
+++ b/node_modules/pdfjs-dist/legacy/web/pdf_viewer.mjs
@@ -9749,10 +9749,16 @@ async function docProperties(pdfDocument) {
 class GenericScripting {
   constructor(sandboxBundleSrc) {
     this._ready = new Promise((resolve, reject) => {
-      const sandbox = import( /*webpackIgnore: true*/sandboxBundleSrc);
-      sandbox.then(pdfjsSandbox => {
-        resolve(pdfjsSandbox.QuickJSSandbox());
-      }).catch(reject);
+      const script = document.createElement('script');
+      script.src = sandboxBundleSrc ?? "https://unpkg.com/[email protected]/build/pdf.sandbox.min.mjs";
+      script.type = 'module';
+      script.onload = () => {
+        import(sandboxBundleSrc).then(pdfjsSandbox => {
+          resolve(pdfjsSandbox.QuickJSSandbox());
+        }).catch(reject);
+      };
+      script.onerror = reject;
+      document.head.appendChild(script);
     });
   }
   async createSandbox(data) {
diff --git a/node_modules/pdfjs-dist/web/pdf_viewer.css b/node_modules/pdfjs-dist/web/pdf_viewer.css
index 94828c8..e4065fc 100644
--- a/node_modules/pdfjs-dist/web/pdf_viewer.css
+++ b/node_modules/pdfjs-dist/web/pdf_viewer.css
@@ -315,14 +315,14 @@
               backdrop-filter:var(--highlight-selected-backdrop-filter);
     }
 
-.textLayer ::-moz-selection{
-    background:rgba(0 0 255 / 0.25);
-    background:color-mix(in srgb, AccentColor, transparent 75%);
+.textLayer ::-moz-selection {
+    background: blue;
+    background: AccentColor;
   }
   
-.textLayer ::selection{
-    background:rgba(0 0 255 / 0.25);
-    background:color-mix(in srgb, AccentColor, transparent 75%);
+.textLayer ::selection {
+    background: blue;
+    background: AccentColor;
   }
 
 .textLayer br::-moz-selection{
diff --git a/node_modules/pdfjs-dist/web/pdf_viewer.mjs b/node_modules/pdfjs-dist/web/pdf_viewer.mjs
index 9c4f97d..35f1ce4 100644
--- a/node_modules/pdfjs-dist/web/pdf_viewer.mjs
+++ b/node_modules/pdfjs-dist/web/pdf_viewer.mjs
@@ -5994,10 +5994,16 @@ async function docProperties(pdfDocument) {
 class GenericScripting {
   constructor(sandboxBundleSrc) {
     this._ready = new Promise((resolve, reject) => {
-      const sandbox = import( /*webpackIgnore: true*/sandboxBundleSrc);
-      sandbox.then(pdfjsSandbox => {
-        resolve(pdfjsSandbox.QuickJSSandbox());
-      }).catch(reject);
+      const script = document.createElement('script');
+      script.src = sandboxBundleSrc ?? "https://unpkg.com/[email protected]/build/pdf.sandbox.min.mjs";
+      script.type = 'module';
+      script.onload = () => {
+        import(sandboxBundleSrc).then(pdfjsSandbox => {
+          resolve(pdfjsSandbox.QuickJSSandbox());
+        }).catch(reject);
+      };
+      script.onerror = reject;
+      document.head.appendChild(script);
     });
   }
   async createSandbox(data) {

Basically, it didn't respect the /* webpack-ignore */ comments and there was an issue in LightningCSS which causes color-mix with system colors to fail.

parcel-bundler/lightningcss#685 (comment)

Not sure if you want to include these patches in your library as well, but just wanted to flag!

@agentcooper
Copy link
Owner

@0kzh Wrong type imports might be because I forgot to update package.json main: 3004b57.

I am not familiar with Turbopack, but any chance you have a minimal repro for the Next.js setup?

@agentcooper agentcooper merged commit 705d11d into agentcooper:main Sep 9, 2024
1 check passed
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.

Next.js support (use CSS modules)
2 participants