Skip to content

enhancement/issue 823 ensure greenwood cli and plugins are favoring idiomatic usages of async #1546

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
thescientist13 merged 18 commits into from
Aug 23, 2025
Merged

enhancement/issue 823 ensure greenwood cli and plugins are favoring idiomatic usages of async #1546

Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
a805a36
chore: refactor async callbacks
KaiPrince Aug 3, 2025
5aa3439
chore: remove promise.resolve/reject in async functions
KaiPrince Aug 4, 2025
2a8fcf8
Revert "chore: remove promise.resolve/reject in async functions"
KaiPrince Aug 4, 2025
1233214
refactor one function
KaiPrince Aug 7, 2025
f3575b8
use clone consistently
KaiPrince Aug 11, 2025
b730cbd
Revert "refactor one function"
KaiPrince Aug 11, 2025
4ecc2bf
Update asyncMap
KaiPrince Aug 11, 2025
5674288
use asyncMap
KaiPrince Aug 11, 2025
b65ee02
use asyncForEach
KaiPrince Aug 16, 2025
6fcbf11
Merge remote-tracking branch 'upstream/master' into enhancement/issue…
KaiPrince Aug 19, 2025
41e0acc
Add eslint await in loop rule
KaiPrince Aug 19, 2025
7e3f0c8
fix: revert merged response .clone cleanup
KaiPrince Aug 21, 2025
a5af458
use `return await` for better stack trace
KaiPrince Aug 21, 2025
2159832
await forEach
KaiPrince Aug 21, 2025
80afa74
await more forEach
KaiPrince Aug 21, 2025
2a591b0
revert no await in loop eslint rule
KaiPrince Aug 21, 2025
7f2339a
Merge branch 'master' into enhancement/issue-823-ensure-Greenwood-CLI…
KaiPrince Aug 21, 2025
6da1e3b
add return await for better stack trace
KaiPrince Aug 21, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions eslint.config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ export default [
"no-only-tests/no-only-tests": "error",
"import/no-unresolved": "off",
"import/enforce-node-protocol-usage": ["error", "always"],
"no-await-in-loop": "warn",

@thescientist13 thescientist13 Aug 21, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

my concern with adding this rule with no "constraint" on this is that now we get a ton of warnings

Details 
    /Users/owenbuckley/Workspace/project-evergreen/greenwood/packages/cli/src/commands/eject.js
  11:5  warning  Unexpected `await` inside a loop  no-await-in-loop

/Users/owenbuckley/Workspace/project-evergreen/greenwood/packages/cli/src/config/rollup.config.js

78:42  warning  Unexpected await inside a loop  no-await-in-loop

79:32  warning  Unexpected await inside a loop  no-await-in-loop

90:38  warning  Unexpected await inside a loop  no-await-in-loop

91:24  warning  Unexpected await inside a loop  no-await-in-loop

98:14  warning  Unexpected await inside a loop  no-await-in-loop

100:24  warning  Unexpected await inside a loop  no-await-in-loop

107:14  warning  Unexpected await inside a loop  no-await-in-loop

109:24  warning  Unexpected await inside a loop  no-await-in-loop

114:41  warning  Unexpected await inside a loop  no-await-in-loop

115:24  warning  Unexpected await inside a loop  no-await-in-loop

145:38  warning  Unexpected await inside a loop  no-await-in-loop

287:40  warning  Unexpected await inside a loop  no-await-in-loop

288:30  warning  Unexpected await inside a loop  no-await-in-loop

293:38  warning  Unexpected await inside a loop  no-await-in-loop

294:24  warning  Unexpected await inside a loop  no-await-in-loop

302:14  warning  Unexpected await inside a loop  no-await-in-loop

304:24  warning  Unexpected await inside a loop  no-await-in-loop

312:14  warning  Unexpected await inside a loop  no-await-in-loop

314:24  warning  Unexpected await inside a loop  no-await-in-loop

357:31  warning  Unexpected await inside a loop  no-await-in-loop

363:38  warning  Unexpected await inside a loop  no-await-in-loop

364:30  warning  Unexpected await inside a loop  no-await-in-loop

637:22  warning  Unexpected await inside a loop  no-await-in-loop

639:32  warning  Unexpected await inside a loop  no-await-in-loop

646:22  warning  Unexpected await inside a loop  no-await-in-loop

648:32  warning  Unexpected await inside a loop  no-await-in-loop

655:22  warning  Unexpected await inside a loop  no-await-in-loop

657:32  warning  Unexpected await inside a loop  no-await-in-loop

663:19  warning  Unexpected await inside a loop  no-await-in-loop


/Users/owenbuckley/Workspace/project-evergreen/greenwood/packages/cli/src/lib/async-utils.js

6:21  warning  Unexpected await inside a loop  no-await-in-loop

21:22  warning  Unexpected await inside a loop  no-await-in-loop


/Users/owenbuckley/Workspace/project-evergreen/greenwood/packages/cli/src/lib/layout-utils.js

23:11  warning  Unexpected await inside a loop  no-await-in-loop

165:40  warning  Unexpected await inside a loop  no-await-in-loop

198:40  warning  Unexpected await inside a loop  no-await-in-loop


/Users/owenbuckley/Workspace/project-evergreen/greenwood/packages/cli/src/lib/resource-utils.js

114:9  warning  Unexpected await inside a loop  no-await-in-loop


/Users/owenbuckley/Workspace/project-evergreen/greenwood/packages/cli/src/lib/walker-package-ranger.js

235:15  warning  Unexpected await inside a loop  no-await-in-loop

255:11  warning  Unexpected await inside a loop  no-await-in-loop

300:14  warning  Unexpected await inside a loop  no-await-in-loop

304:11  warning  Unexpected await inside a loop  no-await-in-loop

309:13  warning  Unexpected await inside a loop  no-await-in-loop


/Users/owenbuckley/Workspace/project-evergreen/greenwood/packages/cli/src/lifecycles/bundle.js

26:39  warning  Unexpected await inside a loop  no-await-in-loop

27:18  warning  Unexpected await inside a loop  no-await-in-loop

30:36  warning  Unexpected await inside a loop  no-await-in-loop

31:18  warning  Unexpected await inside a loop  no-await-in-loop

44:35  warning  Unexpected await inside a loop  no-await-in-loop

45:50  warning  Unexpected await inside a loop  no-await-in-loop

110:7   warning  Unexpected await inside a loop  no-await-in-loop

144:41  warning  Unexpected await inside a loop  no-await-in-loop

145:37  warning  Unexpected await inside a loop  no-await-in-loop

193:13  warning  Unexpected await inside a loop  no-await-in-loop

194:9   warning  Unexpected await inside a loop  no-await-in-loop

205:22  warning  Unexpected await inside a loop  no-await-in-loop

224:18  warning  Unexpected await inside a loop  no-await-in-loop

249:18  warning  Unexpected await inside a loop  no-await-in-loop

274:18  warning  Unexpected await inside a loop  no-await-in-loop

284:31  warning  Unexpected await inside a loop  no-await-in-loop

292:7   warning  Unexpected await inside a loop  no-await-in-loop

305:22  warning  Unexpected await inside a loop  no-await-in-loop

306:7   warning  Unexpected await inside a loop  no-await-in-loop

333:20  warning  Unexpected await inside a loop  no-await-in-loop

334:20  warning  Unexpected await inside a loop  no-await-in-loop

335:20  warning  Unexpected await inside a loop  no-await-in-loop

337:20  warning  Unexpected await inside a loop  no-await-in-loop

338:9   warning  Unexpected await inside a loop  no-await-in-loop

346:7   warning  Unexpected await inside a loop  no-await-in-loop

372:20  warning  Unexpected await inside a loop  no-await-in-loop

373:9   warning  Unexpected await inside a loop  no-await-in-loop

377:13  warning  Unexpected await inside a loop  no-await-in-loop

378:9   warning  Unexpected await inside a loop  no-await-in-loop

387:7   warning  Unexpected await inside a loop  no-await-in-loop

433:24  warning  Unexpected await inside a loop  no-await-in-loop

434:9   warning  Unexpected await inside a loop  no-await-in-loop


/Users/owenbuckley/Workspace/project-evergreen/greenwood/packages/cli/src/lifecycles/copy.js

46:30  warning  Unexpected await inside a loop  no-await-in-loop

48:30  warning  Unexpected await inside a loop  no-await-in-loop

49:11  warning  Unexpected await inside a loop  no-await-in-loop

53:11  warning  Unexpected await inside a loop  no-await-in-loop

67:23  warning  Unexpected await inside a loop  no-await-in-loop

73:9   warning  Unexpected await inside a loop  no-await-in-loop

75:9   warning  Unexpected await inside a loop  no-await-in-loop


/Users/owenbuckley/Workspace/project-evergreen/greenwood/packages/cli/src/lifecycles/graph.js

73:10  warning  Unexpected await inside a loop  no-await-in-loop

74:10  warning  Unexpected await inside a loop  no-await-in-loop

77:27  warning  Unexpected await inside a loop  no-await-in-loop

91:38  warning  Unexpected await inside a loop  no-await-in-loop

110:33  warning  Unexpected await inside a loop  no-await-in-loop

162:28  warning  Unexpected await inside a loop  no-await-in-loop

175:29  warning  Unexpected await inside a loop  no-await-in-loop

178:13  warning  Unexpected await inside a loop  no-await-in-loop

347:20  warning  Unexpected await inside a loop  no-await-in-loop


/Users/owenbuckley/Workspace/project-evergreen/greenwood/packages/cli/src/lifecycles/prerender.js

23:32  warning  Unexpected await inside a loop  no-await-in-loop

24:18  warning  Unexpected await inside a loop  no-await-in-loop

40:8   warning  Unexpected await inside a loop  no-await-in-loop

42:42  warning  Unexpected await inside a loop  no-await-in-loop

45:36  warning  Unexpected await inside a loop  no-await-in-loop

46:42  warning  Unexpected await inside a loop  no-await-in-loop

92:16  warning  Unexpected await inside a loop  no-await-in-loop

92:23  warning  Unexpected await inside a loop  no-await-in-loop

93:12  warning  Unexpected await inside a loop  no-await-in-loop

93:19  warning  Unexpected await inside a loop  no-await-in-loop

112:23  warning  Unexpected await inside a loop  no-await-in-loop

117:12  warning  Unexpected await inside a loop  no-await-in-loop

142:5   warning  Unexpected await inside a loop  no-await-in-loop

143:5   warning  Unexpected await inside a loop  no-await-in-loop


/Users/owenbuckley/Workspace/project-evergreen/greenwood/packages/cli/src/lifecycles/serve.js

70:36  warning  Unexpected await inside a loop  no-await-in-loop

71:27  warning  Unexpected await inside a loop  no-await-in-loop


/Users/owenbuckley/Workspace/project-evergreen/greenwood/packages/cli/src/loader.js

36:8   warning  Unexpected await inside a loop  no-await-in-loop

41:24  warning  Unexpected await inside a loop  no-await-in-loop

47:32  warning  Unexpected await inside a loop  no-await-in-loop

51:44  warning  Unexpected await inside a loop  no-await-in-loop

59:8   warning  Unexpected await inside a loop  no-await-in-loop

66:11  warning  Unexpected await inside a loop  no-await-in-loop

71:36  warning  Unexpected await inside a loop  no-await-in-loop

75:44  warning  Unexpected await inside a loop  no-await-in-loop


/Users/owenbuckley/Workspace/project-evergreen/greenwood/packages/cli/src/plugins/resource/plugin-standard-html.js

74:31  warning  Unexpected await inside a loop  no-await-in-loop


/Users/owenbuckley/Workspace/project-evergreen/greenwood/packages/cli/test/cases/build.plugins.adapter/generic-adapter.js

36:21  warning  Unexpected await inside a loop  no-await-in-loop

40:5   warning  Unexpected await inside a loop  no-await-in-loop

43:7   warning  Unexpected await inside a loop  no-await-in-loop

53:5   warning  Unexpected await inside a loop  no-await-in-loop

58:7   warning  Unexpected await inside a loop  no-await-in-loop


/Users/owenbuckley/Workspace/project-evergreen/greenwood/packages/plugin-adapter-aws/src/index.js

84:21  warning  Unexpected await inside a loop  no-await-in-loop

88:5   warning  Unexpected await inside a loop  no-await-in-loop

91:5   warning  Unexpected await inside a loop  no-await-in-loop

99:7   warning  Unexpected await inside a loop  no-await-in-loop

111:5   warning  Unexpected await inside a loop  no-await-in-loop

113:5   warning  Unexpected await inside a loop  no-await-in-loop

118:7   warning  Unexpected await inside a loop  no-await-in-loop


/Users/owenbuckley/Workspace/project-evergreen/greenwood/packages/plugin-adapter-netlify/src/index.js

107:21  warning  Unexpected await inside a loop  no-await-in-loop

112:5   warning  Unexpected await inside a loop  no-await-in-loop

115:5   warning  Unexpected await inside a loop  no-await-in-loop

123:7   warning  Unexpected await inside a loop  no-await-in-loop

128:5   warning  Unexpected await inside a loop  no-await-in-loop

149:5   warning  Unexpected await inside a loop  no-await-in-loop

151:5   warning  Unexpected await inside a loop  no-await-in-loop

156:7   warning  Unexpected await inside a loop  no-await-in-loop

161:5   warning  Unexpected await inside a loop  no-await-in-loop


/Users/owenbuckley/Workspace/project-evergreen/greenwood/packages/plugin-adapter-vercel/src/index.js

100:21  warning  Unexpected await inside a loop  no-await-in-loop

104:5   warning  Unexpected await inside a loop  no-await-in-loop

107:5   warning  Unexpected await inside a loop  no-await-in-loop

115:7   warning  Unexpected await inside a loop  no-await-in-loop

127:5   warning  Unexpected await inside a loop  no-await-in-loop

129:5   warning  Unexpected await inside a loop  no-await-in-loop

134:7   warning  Unexpected await inside a loop  no-await-in-loop


/Users/owenbuckley/Workspace/project-evergreen/greenwood/packages/plugin-adapter-vercel/test/cases/build.config.base-path/build.config.base-path.spec.js

119:28  warning  Unexpected await inside a loop  no-await-in-loop


/Users/owenbuckley/Workspace/project-evergreen/greenwood/packages/plugin-adapter-vercel/test/cases/build.default.options-runtime/build.default.options-runtime.spec.js

119:28  warning  Unexpected await inside a loop  no-await-in-loop


/Users/owenbuckley/Workspace/project-evergreen/greenwood/packages/plugin-adapter-vercel/test/cases/build.default/build.default.spec.js

135:28  warning  Unexpected await inside a loop  no-await-in-loop


/Users/owenbuckley/Workspace/project-evergreen/greenwood/packages/plugin-css-modules/src/index.js

51:32  warning  Unexpected await inside a loop  no-await-in-loop

52:18  warning  Unexpected await inside a loop  no-await-in-loop

59:8   warning  Unexpected await inside a loop  no-await-in-loop

61:18  warning  Unexpected await inside a loop  no-await-in-loop

68:8   warning  Unexpected await inside a loop  no-await-in-loop

70:18  warning  Unexpected await inside a loop  no-await-in-loop

168:5   warning  Unexpected await inside a loop  no-await-in-loop

222:11  warning  Unexpected await inside a loop  no-await-in-loop


/Users/owenbuckley/Workspace/project-evergreen/greenwood/packages/plugin-graphql/src/schema/schema.js

43:53  warning  Unexpected await inside a loop  no-await-in-loop


/Users/owenbuckley/Workspace/project-evergreen/greenwood/packages/plugin-include-html/src/index.js

29:33  warning  Unexpected await inside a loop  no-await-in-loop

50:42  warning  Unexpected await inside a loop  no-await-in-loop

51:33  warning  Unexpected await inside a loop  no-await-in-loop

51:51  warning  Unexpected await inside a loop  no-await-in-loop


/Users/owenbuckley/Workspace/project-evergreen/greenwood/packages/plugin-renderer-lit/src/execute-route-module.js

26:7  warning  Unexpected await inside a loop  no-await-in-loop

What I was trying to callout in my comment was that using await in a for ... of is fine. So unless we can limit the linting to just map, filter, etc then I worry about all the extra noise this adds. The only other option is to add an eslint ignore to all those locations, which seemed pretty tedious.

@thescientist13 thescientist13 Aug 21, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm, I realize i kind of left an unfinished sentence here, my apologies 😞

I would be open to adding the linting rule as I think most of the violations flagged are probably just going to be in for..of loops and so I think those are fine to add an ESLint ignore line for

So yeah, my above comment was what I trying to say, so sorry about that mixup

@KaiPrince KaiPrince Aug 21, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah it's a lot. That's what I was trying to say in my other comment. We'd have to add eslint-disable comments to a lot of places, or just leave it as a warn (which as you say can get pretty noisy).

I thought you confirmed you wanted it for keeping us "honest", but no problem I'll just revert this config change for now. If we decide later we want to add all those eslint-ignore comments, I can open a separate PR.

@thescientist13 thescientist13 Aug 21, 2025 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, apologies I didn't finish my full thought in that most recent comment, but I was trying to say if an eslint rule is added, it should (ideally) be done with the ability to carve out an exception for the use cases we do want it for (e.g. for...of, for..in)

So the options would be

  1. Enable with error and add // eslint-disable-line no-await-in-loop everywhere its allowed (or see if we can just have it error on map, filter, etc
  2. Just leave things as they are (and just fix the merge conflict, which you done, which is great!)

So I was leaning towards option #2 depending if the carve out was feasible, or if we wanted to sprinkle in all those ignore lines.

Thoughts?

Like I said I am good with things right now, I've personally broken the habit of the async maps, filters etc so I am at least trying to keep myself honest now 😇

Also, you might want to do one more rebase since I bumped the Node version and so GitHub Actions checks have been updated accordingly. Then we can run the final checks on your PR. 👍

@KaiPrince KaiPrince Aug 21, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I started adding eslint-ignore comments initially, but I think I hit a snag in the rollup config file. Let's do option 2 for now, and I'll start a follow-up PR to see where option 1 takes us.

PR has been updated from master 👍

},
plugins: {
"no-only-tests": noOnlyTests,
Expand Down
8 changes: 1 addition & 7 deletions packages/cli/src/commands/build.js
View file
Open in desktop

@KaiPrince KaiPrince Aug 4, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I recommend hiding whitespace changes for easier reviewing

Original file line number Diff line number Diff line change
Expand Up @@ -38,13 +38,7 @@ const runProductionBuild = async (compilation) => {
});
}

await Promise.all(
servers.map(async (server) => {
await server.start();

return Promise.resolve(server);
}),
);
await Promise.all(servers.map((server) => server.start()));

@thescientist13 thescientist13 Aug 5, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice, this looks very elegant! 🤓


if (prerenderPlugin.executeModuleUrl) {
await preRenderCompilationWorker(compilation, prerenderPlugin);
Expand Down
18 changes: 12 additions & 6 deletions packages/cli/src/lib/async-utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,27 +1,33 @@
// https://stackoverflow.com/a/76974728/417806
// Constraint: callback functions must not depend on each other
async function asyncFilter(arr, cb) {
const filtered = [];

for (const element of arr) {
await asyncForEach(arr, async (element) => {
const needAdd = await cb(element);

if (needAdd) {
filtered.push(element);
}
}
});

return filtered;
}

// https://stackoverflow.com/a/71278238/417806
// Constraint: mapper functions must not depend on each other

@KaiPrince KaiPrince Aug 11, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Documented a constraint here, since the callbacks need to be safe to run concurrently.

async function asyncMap(items, mapper) {
const mappedItems = [];
const promises = [];

for (const item of items) {
mappedItems.push(await mapper(item));
promises.push(mapper(item));
Comment on lines -21 to +23

@KaiPrince KaiPrince Aug 11, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using Promise.all rather than awaiting in the for loop allows them to run in parallel. This technique also matches the stack overflow source linked in the comment.

}

return mappedItems;
return Promise.all(promises);
}

async function asyncForEach(items, callback) {

@thescientist13 thescientist13 Aug 21, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I think I must have missed this in a previous review but why would we need a custom asyncForEach? Wouldn't a for..or and / or for ... in achieve the same result, without needing a custom utility?

@KaiPrince KaiPrince Aug 21, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the difference is that this wraps them in Promise.all, which allows each callback to run concurrently for better performance.

Often, the code can be refactored to create all the promises at once, then get access to the results using Promise.all() (or one of the other promise concurrency methods). Otherwise, each successive operation will not start until the previous one has completed.

https://eslint.org/docs/latest/rules/no-await-in-loop

This video is a good recap of async/await: https://youtu.be/vn3tm0quoqE?t=341. I recommend you watch it to make sure we're on the same page.

@thescientist13 thescientist13 Aug 21, 2025 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah yes, I think you may have mentioned that before and it just passed me by. 🤦‍♂️

So basically in cases where we want to run a bunch of independent tasks that don't depend on each other, we can run them concurrently using your new asyncForEach function. As opposed to a situation in the case of running some Greenwood plugins over some code / files to transform them in order, that's where using a for ... of / for ... in would be more appropriate.

Good stuff, thanks for taking the time to explain your approach here, much appreciated! 🙌

@thescientist13 thescientist13 Aug 21, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That video link was great btw, Fireship is the best! 🔥

await asyncMap(items, callback);
}

export { asyncFilter, asyncMap };
export { asyncFilter, asyncMap, asyncForEach };
167 changes: 68 additions & 99 deletions packages/cli/src/lifecycles/bundle.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import {
import path from "node:path";
import { rollup } from "rollup";
import { pruneGraph } from "../lib/content-utils.js";
import { asyncForEach } from "../lib/async-utils.js";

async function interceptPage(url, request, plugins, body) {
let response = new Response(body, {
Expand Down Expand Up @@ -101,65 +102,61 @@ async function emitResources(compilation) {
async function cleanUpResources(compilation) {
const { outputDir } = compilation.context;

for (const resource of compilation.resources.values()) {
asyncForEach(compilation.resources.values(), async (resource) => {
const { src, optimizedFileName, optimizationAttr } = resource;
const optConfig = ["inline", "static"].indexOf(compilation.config.optimization) >= 0;
const optAttr = ["inline", "static"].indexOf(optimizationAttr) >= 0;

if (optimizedFileName && (!src || optAttr || optConfig)) {
await fs.unlink(new URL(`./${optimizedFileName}`, outputDir));
}
}
});
}

async function optimizeStaticPages(compilation, plugins) {
const { scratchDir, outputDir } = compilation.context;

return Promise.all(
compilation.graph
.filter(
(page) =>
!page.isSSR ||
(page.isSSR && page.prerender) ||
(page.isSSR && compilation.config.prerender),
)
.map(async (page) => {
const { route, outputHref } = page;
const outputDirUrl = new URL(outputHref.replace("index.html", "").replace("404.html", ""));
const url = new URL(`http://localhost:${compilation.config.port}${route}`);
const contents = await fs.readFile(
new URL(`./${outputHref.replace(outputDir.href, "")}`, scratchDir),
"utf-8",
);
const headers = new Headers({ "Content-Type": "text/html" });
let response = new Response(contents, { headers });

if (!(await checkResourceExists(outputDirUrl))) {
await fs.mkdir(outputDirUrl, {
recursive: true,
});
}
const pages = compilation.graph.filter(
(page) =>
!page.isSSR || (page.isSSR && page.prerender) || (page.isSSR && compilation.config.prerender),

@KaiPrince KaiPrince Aug 11, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's an unintentional whitespace change here, but the linter requires this change.

);

for (const plugin of plugins) {
if (plugin.shouldOptimize && (await plugin.shouldOptimize(url, response.clone()))) {
const currentResponse = await plugin.optimize(url, response.clone());
await asyncForEach(pages, async (page) => {
const { route, outputHref } = page;
const outputDirUrl = new URL(outputHref.replace("index.html", "").replace("404.html", ""));
const url = new URL(`http://localhost:${compilation.config.port}${route}`);
const contents = await fs.readFile(
new URL(`./${outputHref.replace(outputDir.href, "")}`, scratchDir),
"utf-8",
);
const headers = new Headers({ "Content-Type": "text/html" });
let response = new Response(contents, { headers });

if (!(await checkResourceExists(outputDirUrl))) {
await fs.mkdir(outputDirUrl, {
recursive: true,
});
}

response = mergeResponse(response.clone(), currentResponse.clone());
}
}
for (const plugin of plugins) {
if (plugin.shouldOptimize && (await plugin.shouldOptimize(url, response.clone()))) {
const currentResponse = await plugin.optimize(url, response.clone());

// clean up optimization markers
const body = (await response.text()).replace(/data-gwd-opt=".*?[a-z]"/g, "");
response = mergeResponse(response.clone(), currentResponse.clone());
}
}

await fs.writeFile(new URL(outputHref), body);
}),
);
// clean up optimization markers
const body = (await response.text()).replace(/data-gwd-opt=".*?[a-z]"/g, "");

await fs.writeFile(new URL(outputHref), body);
});
}

async function bundleStyleResources(compilation, resourcePlugins) {
const { outputDir } = compilation.context;

for (const resource of compilation.resources.values()) {
await asyncForEach(compilation.resources.values(), async (resource) => {
const { contents, src = "", type } = resource;

if (["style", "link"].includes(type)) {
Expand Down Expand Up @@ -202,84 +199,58 @@ async function bundleStyleResources(compilation, resourcePlugins) {
const request = new Request(url, { headers });
const initResponse = new Response(contents, { headers });

let response = await resourcePlugins.reduce(async (responsePromise, plugin) => {
const intermediateResponse = await responsePromise;
let response = initResponse;

for (const plugin of resourcePlugins) {
Comment thread
KaiPrince marked this conversation as resolved.
const shouldServe = plugin.shouldServe && (await plugin.shouldServe(url, request));

if (shouldServe) {
const currentResponse = await plugin.serve(url, request);
const mergedResponse = mergeResponse(
intermediateResponse.clone(),
currentResponse.clone(),
);
const mergedResponse = mergeResponse(response.clone(), currentResponse.clone());

if (mergedResponse.headers.get("Content-Type").indexOf(contentType) >= 0) {
return Promise.resolve(mergedResponse.clone());
response = mergedResponse.clone();
}
}
}

return Promise.resolve(responsePromise);
}, Promise.resolve(initResponse));

response = await resourcePlugins.reduce(async (responsePromise, plugin) => {
const intermediateResponse = await responsePromise;
for (const plugin of resourcePlugins) {
const shouldPreIntercept =
plugin.shouldPreIntercept &&
(await plugin.shouldPreIntercept(url, request, intermediateResponse.clone()));
(await plugin.shouldPreIntercept(url, request, response.clone()));

if (shouldPreIntercept) {
const currentResponse = await plugin.preIntercept(
url,
request,
intermediateResponse.clone(),
);
const mergedResponse = mergeResponse(
intermediateResponse.clone(),
currentResponse.clone(),
);
const currentResponse = await plugin.preIntercept(url, request, response.clone());
const mergedResponse = mergeResponse(response.clone(), currentResponse.clone());

if (mergedResponse.headers.get("Content-Type").indexOf(contentType) >= 0) {
return Promise.resolve(mergedResponse.clone());
response = mergedResponse.clone();
}
}
}

return Promise.resolve(responsePromise);
}, Promise.resolve(response.clone()));

response = await resourcePlugins.reduce(async (responsePromise, plugin) => {
const intermediateResponse = await responsePromise;
for (const plugin of resourcePlugins) {
const shouldIntercept =
plugin.shouldIntercept &&
(await plugin.shouldIntercept(url, request, intermediateResponse.clone()));
plugin.shouldIntercept && (await plugin.shouldIntercept(url, request, response.clone()));

if (shouldIntercept) {
const currentResponse = await plugin.intercept(
url,
request,
intermediateResponse.clone(),
);
const mergedResponse = mergeResponse(
intermediateResponse.clone(),
currentResponse.clone(),
);
const currentResponse = await plugin.intercept(url, request, response.clone());
const mergedResponse = mergeResponse(response.clone(), currentResponse.clone());

if (mergedResponse.headers.get("Content-Type").indexOf(contentType) >= 0) {
return Promise.resolve(mergedResponse.clone());
response = mergedResponse.clone();
}
}
}

return Promise.resolve(responsePromise);
}, Promise.resolve(response.clone()));

response = await resourcePlugins.reduce(async (responsePromise, plugin) => {
const intermediateResponse = await responsePromise;
for (const plugin of resourcePlugins) {
const shouldOptimize =
plugin.shouldOptimize && (await plugin.shouldOptimize(url, intermediateResponse.clone()));
plugin.shouldOptimize && (await plugin.shouldOptimize(url, response.clone()));

return shouldOptimize
? Promise.resolve(await plugin.optimize(url, intermediateResponse.clone()))
: Promise.resolve(responsePromise);
}, Promise.resolve(response.clone()));
if (shouldOptimize) {
response = await plugin.optimize(url, response.clone());
}
}

optimizedFileContents = await response.text();

Expand All @@ -291,7 +262,7 @@ async function bundleStyleResources(compilation, resourcePlugins) {

await fs.writeFile(new URL(`./${optimizedFileName}`, outputDir), optimizedFileContents);
}
}
});
}

async function bundleApiRoutes(compilation) {
Expand All @@ -300,11 +271,10 @@ async function bundleApiRoutes(compilation) {

if (apiConfigs.length > 0 && apiConfigs[0].input.length !== 0) {
console.info("bundling API routes...");
for (const configIndex in apiConfigs) {
const rollupConfig = apiConfigs[configIndex];
await asyncForEach(apiConfigs, async (rollupConfig) => {
Comment on lines -303 to +274

@KaiPrince KaiPrince Aug 21, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I double-checked, and apiConfigs is an array anyways, so converting from for...in to for...of works fine. No special handling was needed for accessing by index.

const bundle = await rollup(rollupConfig);
await bundle.write(rollupConfig.output);
}
});
}
}

Expand All @@ -326,7 +296,7 @@ async function bundleSsrPages(compilation, optimizePlugins) {
// one pass to generate initial static HTML and to track all combined static resources across layouts
// and before we optimize so that all bundled assets can tracked up front
// would be nice to see if this can be done in a single pass though...
for (const page of ssrPages) {
await asyncForEach(ssrPages, async (page) => {
const { route } = page;
let staticHtml = "<content-outlet></content-outlet>";

Expand All @@ -346,15 +316,15 @@ async function bundleSsrPages(compilation, optimizePlugins) {
await trackResourcesForRoute(staticHtml, compilation, route);

ssrPrerenderPagesRouteMapper[route] = staticHtml;
}
});

// technically this happens in the start of bundleCompilation once
// so might be nice to detect those static assets to see if they have be "de-duped" from bundling here
await bundleScriptResources(compilation);
await bundleStyleResources(compilation, optimizePlugins);

// second pass to link all bundled assets to their resources before optimizing and generating SSR bundles
for (const page of ssrPages) {
await asyncForEach(ssrPages, async (page) => {
const { id, route, pageHref } = page;
const pagePath = new URL(pageHref).pathname.replace(pagesDir.pathname, "./");
const entryFileUrl = new URL(pageHref);
Expand Down Expand Up @@ -422,17 +392,16 @@ async function bundleSsrPages(compilation, optimizePlugins) {
id,
inputPath: normalizePathnameForWindows(entryFileOutputUrl),
});
}
});

const ssrConfigs = await getRollupConfigForSsrPages(compilation, input);

if (ssrConfigs.length > 0 && ssrConfigs[0].input !== "") {
console.info("bundling dynamic pages...");
for (const configIndex in ssrConfigs) {
const rollupConfig = ssrConfigs[configIndex];
await asyncForEach(ssrConfigs, async (rollupConfig) => {
const bundle = await rollup(rollupConfig);
await bundle.write(rollupConfig.output);
}
});
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion packages/cli/src/lifecycles/compile.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ const generateCompilation = async () => {

if (!(await checkResourceExists(new URL("./graph.json", outputDir)))) {
return Promise.reject(
new Error("No build output detected. Make sure you have run greenwood build"),
new Error("No build output detected. Make sure you have run greenwood build"),
);
}

Expand Down
Loading