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

Replace calls to eval('require') with require #213

Open
michaelhays opened this issue May 30, 2024 · 13 comments
Open

Replace calls to eval('require') with require #213

michaelhays opened this issue May 30, 2024 · 13 comments
Assignees
Labels
bug Something isn't working dontstale Mark issues not to be removed by stalebot

Comments

@michaelhays
Copy link

Description of the Issue

I've had issues running this library on Vercel serverless functions for a few months now, documented in #54 and #96.

I just upgraded to [email protected] and am still getting the same issue, with a third library this time:

Cannot find module 'proxy-agent'

However, this time I tried patching the library, and replacing all calls to eval('require') with simply require (such as on this line) and this seems to have fixed all of my issues.

Are you able to do this across this codebase? Curious, what was the reason for using eval('require') in the first place?

Versions Used

Typescript SDK: v1.0.0
Platform: Node.js
Node.js (if applicable): 20.13.1

@congminh1254
Copy link
Member

Hi @michaelhays

The reason why we are using this eval('require') instead of require is to make it compatible with some frontend frameworks, while these frameworks are not using the libraries above but still trying to resolve it.

We will soon change it back to require but we need to figure out whether the frontend framework should use polyfills to override the missing library by their side or if we will have a custom version of the TS SDK for the browser only.

We are sorry for any inconvenience.

Best,
Minh

@jasonappah
Copy link

For anyone else dealing with this issue with Next.js on Vercel, there is an config option outputFileTracingIncludes that might help since you can require that certain files/dirs are included with the final build output without relying on automatic tracing. My assumption is that their static tracing can't detect that proxy-agent needs to be included in the final build, perhaps because it isn't a regular require? Haven't tried this just yet but will report back once I do.

Either way, thanks for maintaining this library, and hopefully this can be helpful to anyone else having this issue :)

@michaelhays
Copy link
Author

Success! Thanks so much for the tip @jasonappah.

You need to include the full nested set of dependencies for form-data and proxy-agent, so here's what I had add to my next.config.js file:

/** @type {import('next').NextConfig} */
const config = {
  experimental: {
    outputFileTracingIncludes: {
      '/api/**/*': [
        'node_modules/@tootallnate/quickjs-emscripten/**/*',
        'node_modules/agent-base/**/*',
        'node_modules/ast-types/**/*',
        'node_modules/asynckit/**/*',
        'node_modules/basic-ftp/**/*',
        'node_modules/combined-stream/**/*',
        'node_modules/data-uri-to-buffer/**/*',
        'node_modules/debug/**/*',
        'node_modules/degenerator/**/*',
        'node_modules/delayed-stream/**/*',
        'node_modules/escodegen/**/*',
        'node_modules/esprima/**/*',
        'node_modules/estraverse/**/*',
        'node_modules/esutils/**/*',
        'node_modules/form-data/**/*',
        'node_modules/fs-extra/**/*',
        'node_modules/get-uri/**/*',
        'node_modules/graceful-fs/**/*',
        'node_modules/http-proxy-agent/**/*',
        'node_modules/https-proxy-agent/**/*',
        'node_modules/ip-address/**/*',
        'node_modules/jsbn/**/*',
        'node_modules/jsonfile/**/*',
        'node_modules/lru-cache/**/*',
        'node_modules/mime-db/**/*',
        'node_modules/mime-types/**/*',
        'node_modules/ms/**/*',
        'node_modules/netmask/**/*',
        'node_modules/pac-proxy-agent/**/*',
        'node_modules/pac-resolver/**/*',
        'node_modules/proxy-agent/**/*',
        'node_modules/proxy-from-env/**/*',
        'node_modules/smart-buffer/**/*',
        'node_modules/socks/**/*',
        'node_modules/socks-proxy-agent/**/*',
        'node_modules/sprintf-js/**/*',
        'node_modules/tslib/**/*',
        'node_modules/universalify/**/*',
      ],
    },
  },
}

export default config

Note that for Next.js 14, outputFileTracingIncludes is under the experimental config, though it will be moved to stable in Next.js 15 (see vercel/next.js#68464).

If you're curious, this is the dependency tree I mapped out:
form-data
  asynckit
  combined-stream
    delayed-stream
  mime-types
    mime-db
proxy-agent
  agent-base
    debug
      ms
  debug *
  http-proxy-agent
    agent-base *
    debug *
  https-proxy-agent
    agent-base *
    debug *
  lru-cache
  pac-proxy-agent
    @tootallnate/quickjs-emscripten
    agent-base *
    debug *
    get-uri
      basic-ftp
      data-uri-to-buffer
      fs-extra
        graceful-fs
        jsonfile
          universalify
        universalify *
    http-proxy-agent *
    https-proxy-agent *
    pac-resolver
      degenerator
        ast-types
          tslib
        escodegen
          estraverse
          esutils
          esprima
        esprima *
      netmask
  proxy-from-env
  socks-proxy-agent
    agent-base *
    debug *
    socks
      ip-address
        jsbn
        sprintf-js
      smart-buffer

@namack
Copy link

namack commented Aug 14, 2024

Just wanted to add some more negative downstream effects of using eval('require') vs require. With eval('require') esbuild is not able to traverse the dependencies in the source and therefore does not include these dependencies in the bundled output. The result is the same error - Cannot find module 'proxy-agent'.

@namack
Copy link

namack commented Aug 14, 2024

For anyone struggling with this issue - it's a little hacky, but if you're adventurous you can manually patch the source before building:

Linux

cd ./node_modules/box-typescript-sdk/gen/
find . -type f -exec sed -i "s/eval('require')/require/g" {} +

macOS

cd ./node_modules/box-typescript-sdk/gen/
find . -type f -exec sed -i '' "s/eval('require')/require/g" {} +

Copy link

stale bot commented Nov 8, 2024

This issue has been automatically marked as stale because it has not been updated in the last 30 days. It will be closed if no further activity occurs within the next 7 days. Feel free to reach out or mention Box SDK team member for further help and resources if they are needed.

@stale stale bot added the stale Added to issues should be staled label Nov 8, 2024
@michaelhays
Copy link
Author

!notstale

@congminh1254 congminh1254 added dontstale Mark issues not to be removed by stalebot and removed stale Added to issues should be staled labels Nov 11, 2024
@mtucker
Copy link

mtucker commented Dec 3, 2024

Has anyone had any luck resolving this issue when using pnpm? I believe the workaround above is npm-specific...

@congminh1254
Copy link
Member

Hi @michaelhays

We have just released new version of Box TS SDK, can you try and let us know if it resolves your issue?

@michaelhays
Copy link
Author

michaelhays commented Jan 21, 2025

With v1.11.0, I'm still getting the error: Cannot find module 'proxy-agent', and it looks like the fix from this comment isn't working with that version for some reason, so I'm staying back at v1.10.0 for now.

I think what's happening is that when Next.js bundles everything, it checks for imports and calls to require, but since box-typescript-sdk-gen is loading those libraries dynamically, Next.js doesn't know that the library is needed in the bundle.

@congminh1254
Copy link
Member

Hi @michaelhays

We are using dynamic import for libraries to keep it compatible for frontend framework like React. In the latest release we changed from eval('require') to require to eliminate the warning when build the library only.

@marceleq27
Copy link

marceleq27 commented Jan 24, 2025

I just started exploring your sdk in next.js app (next 15) and seems like there is still a problem:

  ./node_modules/box-typescript-sdk-gen/lib/internal/utils.js:98:12
Module not found: Can't resolve <dynamic>
   96 |         throw new Error('This method is not usable in the browser');
   97 |     }
>  98 |     return require(lib);

I had to add eval or downgrade to 1.10.0 to make it work:

function evalRequire(lib) {
    if (isBrowser()) {
        throw new Error('This method is not usable in the browser');
    }
    return eval('require')(lib);
}

@mjc4thewin
Copy link

Feb 4 and this is still an issue. Trying to deploy to Vercel and can't get around the dynamic loading...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working dontstale Mark issues not to be removed by stalebot
Projects
None yet
Development

No branches or pull requests