ardatan/graphql-toolkit

Why are `require()` calls wrapped in `eval()`?

ehmicky opened this issue ยท 8 comments

Why are dynamic require() calls wrapped in eval()?

Is this to fix a TypeScript type issue?

This creates an issue on Netlify Functions. Netlify Functions crawl Node modules JavaScript AST to look for require() statements. This AST parsing looks for require() function calls, but not inside strings (as this would be error-prone).

Thanks for your help!

Hi @ehmicky,
When you have a compiler or bundler that modifies require like webpack or rollup, it would become a problem. For example, webpack marks it as a critical dependency and puts there its own require which prevents to load external modules there. That's why, we needed that trick to avoid this.
Could you explain what you're trying to do and what is not working as expected?

Thanks @ardatan, I understand now.

Basically we're doing what Webpack and Rollup are doing as well, i.e. crawl dependencies and parse JavaScript AST to find require() statements.

Netlify Functions takes a JavaScript file as input and produce a zip archive as output. That zip archive is meant to be uploaded to AWS Lambda.

We parse JavaScript AST and crawl inside Node dependencies (the code is here) to bundle all the code a file needs before zipping all those into a single archive.

When a library is wrapping the require() statements in an eval(), we fail to detect the dependency, which means it does not get bundled with the rest of the code. This leads to the following bug.

I see so since it is a Node environment, isn't that using CommonJS to load external modules? In that case, you can load graphql-tag-pluck lazily. Otherwise, if we include graphql-tag-pluck, bundle size would be increased so much.

I am not depending on this project directly: one of my users is (@jon301).

Are you suggesting users should add require() statements to fix this problem? This could solve it indeed. Actually it seems like this is what that specific user is doing as a workaround. @jon301 what do you think?

@ehmicky Does this canary version work for you?
0.6.8-alpha-23bbb51.1+23bbb51

Thanks a lot @ardatan for working on this! I am not experiencing this problem myself, this is one of my users. @jon301, does this version work for you?

I can confirm the version 0.6.8-alpha-23bbb51.1+23bbb51 resolves the initial issue :

$ ~/tmp/zip-it-and-ship-it/src/bin.js dist/apps/api /tmp/nx-crusher
Error: In file "/Users/jonjon/work/nx-crusher/dist/apps/api/main.js": Cannot find module 'graphql-tag-pluck/package.json' from '/Users/jonjon/work/nx-crusher/node_modules/graphql-toolkit'
$ yarn add graphql-toolkit@0.6.8-alpha-23bbb51.1+23bbb51
...
...
success Saved 8 new dependencies.
info Direct dependencies
โ””โ”€ graphql-toolkit@0.6.8-alpha-23bbb51.1
info All dependencies
โ”œโ”€ @graphql-toolkit/code-file-loader@0.6.8-alpha-23bbb51.1
โ”œโ”€ @graphql-toolkit/core@0.6.8-alpha-23bbb51.1
โ”œโ”€ @graphql-toolkit/file-loading@0.6.8-alpha-23bbb51.1
โ”œโ”€ @graphql-toolkit/graphql-file-loader@0.6.8-alpha-23bbb51.1
โ”œโ”€ @graphql-toolkit/json-file-loader@0.6.8-alpha-23bbb51.1
โ”œโ”€ @graphql-toolkit/url-loader@0.6.8-alpha-23bbb51.1
โ”œโ”€ graphql-tag-pluck@0.8.7
โ””โ”€ graphql-toolkit@0.6.8-alpha-23bbb51.1
โœจ  Done in 16.99s.
$ ~/tmp/zip-it-and-ship-it/src/bin.js dist/apps/api /tmp/nx-crusher
[
  {
    "path": "/tmp/nx-crusher/main.zip",
    "runtime": "js"
  }
]

Thank you @ehmicky and @ardatan for your help ๐Ÿ‘

Available in 0.6.8 !