misc: convert all internal scripts to ESM #6286
Conversation
Josh-Cena
added
the
pr: maintenance
|
Size Change: 0 B Total Size: 676 kB ℹ️ View Unchanged
|
| run().then( | ||
| () => { | ||
| process.exit(0); | ||
| }, | ||
| (e) => { | ||
| console.error(e.message); | ||
| console.error(e.stack); | ||
| process.exit(1); | ||
| }, | ||
| ); |
There was a problem hiding this comment.
Tip: review with hide whitespace
|
✔️ [V2] 🔨 Explore the source changes: 25825cc 🔍 Inspect the deploy log: https://app.netlify.com/sites/docusaurus-2/deploys/61d912fe7dbb360007183460 😎 Browse the preview: https://deploy-preview-6286--docusaurus-2.netlify.app |
|
⚡️ Lighthouse report for the changes in this PR:
Lighthouse ran on https://deploy-preview-6286--docusaurus-2.netlify.app/ |
| await new Promise((resolve) => { | ||
| setTimeout(resolve, 0); | ||
| }); | ||
| const FeatureRequestsPlugin = (await import('./src/featureRequests/FeatureRequestsPlugin.mjs')).default; |
There was a problem hiding this comment.
🤪 IMHO this looks weird, we should just use createConfig now and put everything inside the function
There was a problem hiding this comment.
Yea, but that creates too much indentation changes... I'd rather use this with the return of less indentation for our massive config object
There was a problem hiding this comment.
I'd rather use this with the return of less indentation for our massive config object
I don't understand what you mean here
Do you see this as a temporary step?
There was a problem hiding this comment.
No, I view this as a long term solution.
const config = {
foo: bar, // two spaces of indentation
};async function createConfig() {
return {
foo: bar, // four spaces of indentation
};
}I'd like to avoid that extra level of indentation at all costs
There was a problem hiding this comment.
I'd 1000% prefer to have an extra indentation than to have this in the long term (which we currently have):
const config = {
foo: bar, // two spaces of indentation
};
async function createConfig()
config.xyz = await abc();
return config;
}
module.exports = createConfig;There was a problem hiding this comment.
Ha, I'm not sure if we are to add more async items in the future. We'll see...
There was a problem hiding this comment.
We can keep it for now, I'll figure out a way to clean this up later
Motivation
ESM looks nicer. Also allows top-level await. Also a POC that the docusaurus.config.js file now supports dynamically importing ES module dependencies.
Seems Jest can't ergonomically import ESM yet, so the
theme-translations/update.jswas not converted.Have you read the Contributing Guidelines on pull requests?
Yes
Test Plan
Ran the scripts locally.