nextstrain / nextstrain.org

The Nextstrain website
https://nextstrain.org
GNU Affero General Public License v3.0
87 stars 49 forks source link

Restore delayed import of ./src/app.js by using a dynamic import() #679

Closed tsibley closed 1 year ago

tsibley commented 1 year ago

The inherently dynamic require() was changed to a static import in the conversion to ES modules¹ with "Use cjs-to-es6 to help conversion" (1963fcd). That broke the intended delayed import, since Node.js starts locating and executing static imports before the actual import statements are reached in the importing module's execution. Use a dynamic import() instead to restore the delayed import.

We delay the import until after setting global.verbose so that calls to utils.verbose() in the codebase respect the server's --verbose option as expected.

The missing utils.verbose() output this caused was first noticed by @jameshadfield.²

¹ https://github.com/nextstrain/nextstrain.org/pull/583

² In https://github.com/nextstrain/nextstrain.org/pull/678 as 191ffa28d04424de593fe02132f5f86bbb5cfcbb, a different proposed fix.

Testing

victorlin commented 1 year ago

Can we merge this ASAP? I'll then modify #678 and merge that.

I think @tsibley signed off for the day, but it seems fine to me for you to merge both of these.

tsibley commented 1 year ago

Thanks for merging!