search

simov / grant

OAuth Proxy
MIT License
4.09k stars 257 forks source link

remove dynamic import statements #231

Closed raphaelschwinger closed 3 years ago

raphaelschwinger commented 3 years ago

Thanks @simov and others for your great work!

Summary

Importing from paths generated at runtime breaks parcel, see (parcel-bundler/parcel#4031), or esbuild, see (evanw/esbuild#480). Building fails or throws a warning like this:

warning: This call to "require" will not be bundled because the argument is not a string literal (surround with a try/catch to silence this warning)
    32 │   var handler = require(`./lib/handler/koa-${version}`)

This can lead to problems at runtime.

simov commented 3 years ago

Hi, I understand that your PR fixes the string literal issue, but does it really solves the issue of not having some of those optional files?

raphaelschwinger commented 3 years ago

Hey @simov, What do you mean with "having some of those optional files"? Require should only be executed for a specific file if the same conditions as before apply.

simov commented 3 years ago

How is parcel or esbuild bundling anything in case they are not trying to resolve all paths eagerly by static analysis? That's how webpack works, it tries to bundle all paths found regardless of logical code structure. So what happens is that webpack throws errors https://github.com/simov/grant/issues/212 for the files that are missing.

raphaelschwinger commented 3 years ago

As far as I understand it esbuild is only bundling stuff it can find by static analysis. Thats why all of those require statements in grant.js get not executed and then it fails at runtime.

simov commented 3 years ago

I did a few tests on my own, fixed in #232

raphaelschwinger commented 3 years ago

@simov thank you :)