Remove usages of require.main #9771
Conversation
| if (require.main !== module) { | ||
| throw new Error('This script is designed to be run as the main process'); | ||
| } | ||
|
|
There was a problem hiding this comment.
This was meant as a safely check, but I think it's safe enough to get rid of it 🙃
| // If there is only one argument and `server.js` is being executed directly, | ||
| // legacy it into the config option. | ||
| if (require.main === module && argv['_'].length === 1) { | ||
| argv['config'] = argv['_'][0]; | ||
| argv['_'] = []; | ||
| } | ||
|
|
There was a problem hiding this comment.
Hopefully it's safe to break this legacy feature now? We certainly don't rely on it for production deployments, and it's been over 6 years since we introduced --config.
There was a problem hiding this comment.
Yeah, I think it's fine to break this. Just to make sure I understand, if someone is in fact using the old syntax then it will break with an error message, right? It won't just silently break?
There was a problem hiding this comment.
No, it will in fact silently break; yargs-parser doesn't do any validation.
All images
|
apps/prairielearn/src/server.js
Outdated
| @@ -2165,7 +2158,7 @@ export async function insertDevUser() { | |||
| await sqldb.queryAsync(adminSql, { user_id }); | |||
| } | |||
|
|
|||
| if (require.main === module && config.startServer) { | |||
There was a problem hiding this comment.
Turns out this is required for our test suite 🙃 I'll add it back for now and figure out something else on the ESM conversion branch.
|
This was ultimately rolled into the ESM conversion. |
There is no direct replacement for
require.mainin ESM modules. There's always https://www.npmjs.com/package/es-main... but I'd rather not drag in another dependency when IMO we never actually really need this check.