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

ESM support #11

Closed
HasmH opened this issue May 8, 2024 · 6 comments
Closed

ESM support #11

HasmH opened this issue May 8, 2024 · 6 comments
Assignees
Labels
bug Something isn't working

Comments

@HasmH
Copy link

HasmH commented May 8, 2024

  • dist/index should read export * from './read-pdf.js'; (add a .js to the end of the import location)
  • import and export in read-pdf.ts needs to be from api.ts (or maybe js), not just from api

Documentation: https://nodejs.org/api/esm.html#mandatory-file-extensions

Originally posted by @AaronSterlingGENEICD in #10 (comment)

Using Node v22:

node
Welcome to Node.js v22.1.0.
Type ".help" for more information.
> import('pdf-text-reader')
Promise {
 <pending>,
 [Symbol(async_id_symbol)]: 25,
 [Symbol(trigger_async_id_symbol)]: 6
}
> Uncaught:
Error [ERR_MODULE_NOT_FOUND]: Cannot find module '/home/hasamh/dev/open-source/zzz/node_modules/pdf-text-reader/dist/read-pdf' imported from /home/hasamh/dev/open-source/zzz/node_modules/pdf-text-reader/dist/index.js
   at finalizeResolution (node:internal/modules/esm/resolve:264:11)
   at moduleResolve (node:internal/modules/esm/resolve:924:10)
   at defaultResolve (node:internal/modules/esm/resolve:1148:11)
   at ModuleLoader.defaultResolve (node:internal/modules/esm/loader:541:12)
   at ModuleLoader.resolve (node:internal/modules/esm/loader:510:25)
   at ModuleLoader.getModuleJob (node:internal/modules/esm/loader:240:38) {
 code: 'ERR_MODULE_NOT_FOUND',
 url: 'file:///home/hasamh/dev/open-source/zzz/node_modules/pdf-text-reader/dist/read-pdf'
}
>

To reproduce:

npm init -y
npm install pdf-text-reader #v5.0.0 installed 
#dynamic import of pdf-text-reader
@electrovir electrovir added the bug Something isn't working label May 8, 2024
@electrovir electrovir self-assigned this May 8, 2024
@electrovir
Copy link
Owner

electrovir commented May 8, 2024

Woops, sorry! I'm still new to ESM support in Node.js.

@electrovir
Copy link
Owner

Despite my TS setup, which is surprisingly resilient to this error, I was able to write a test that fails based on your repro case:

`node --experimental-default-type=module -e "console.log(await import('pdf-text-reader'))"`,

I've now fixed that test and deployed that fix in v5.0.1. Please try that version out!

@AaronSterlingGENEICD
Copy link

Thanks very much. I ran into an issue with the new version of pdfjs-dist that I'll write here in case it helps someone.

pdfjs-dist v4 uses Promise.withResolvers, which is not supported by Node until version 22. AWS Lambda will not add Node 22 runtime until November 2024. To continue using this library on supported runtimes, we downgraded back to pdf-text-reader@4 and added a line into read-pdf.js:

async function readPdfPages({ data, filePath, password, pathToPdfJsDistNodeModule, progressCallback, url, }) {
    const documentLoadingTask = (0, pdfjs_dist_1.getDocument)({
        data,
        isEvalSupported: false, <-- new line here
        url: url || filePath,
        useSystemFonts: true,
        password,
        standardFontDataUrl: pathToPdfJsDistNodeModule
            ? (0, path_1.join)(pathToPdfJsDistNodeModule, 'standard_fonts')
            : undefined,
    });

That workaround was suggested in the vulnerability report, so I believe everything is fine from a security perspective.

Thanks again for addressing this so quickly. We'll upgrade to v5 as soon as we can.

@electrovir
Copy link
Owner

I ran into the same issue with Node versions. I can add an options input to readPdfPages so you can pass in isEvalSupported without modifying the source code.

@electrovir
Copy link
Owner

Actually, looks like I already have that, but it's not being used, woops!

@electrovir
Copy link
Owner

I'll fix here: #12

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants