refactor: extract template string code generation into entries/ modules#259
refactor: extract template string code generation into entries/ modules#259yunus25jmi1 wants to merge 1 commit intocloudflare:mainfrom
Conversation
commit: |
|
@southpolesteve @threepointone @elithrar Kindly review the changes. The workflow is completely green. All checks are passed with 12 successful checks & 3 skipped. |
|
/bigbonk review please |
|
@southpolesteve Bonk workflow was cancelled. View workflow run · To retry, trigger Bonk again. |
c9aedba to
1d34026
Compare
|
@southpolesteve, @james-elicx review the changes. |
Extract 5 entry template generators from index.ts and app-dev-server.ts into packages/vinext/src/entries/: - app-rsc-entry.ts: generateRscEntry() + AppRouterConfig interface - app-ssr-entry.ts: generateSsrEntry() - app-browser-entry.ts: generateBrowserEntry() - pages-server-entry.ts: generateServerEntry() (closure -> explicit params) - pages-client-entry.ts: generateClientEntry() (closure -> explicit params) app-dev-server.ts is now a thin re-export barrel (12 lines, was 3034). index.ts reduced by ~1238 lines (2507, was 3745). Snapshot tests added in tests/entry-templates.test.ts to guard against template drift during future refactors. Closes cloudflare#253
1d34026 to
a75a261
Compare
|
To be honest I think it would probably be best to do this incrementally (starting with snapshots) in PRs rather than an all-at-once PR to avoid the risk of regressions due to our fast pace of change |
|
Thanks @james-elicx ! Could you clarify what you'd like to see as a starting point — are you thinking snapshot tests first as a separate PR, then the extractions follow? Just want to make sure I structure the follow-ups in a way that works for the team's review cadence. |
|
Yeah lets get the snapshot tests in first, then do each of the extractions after separately so that we can have strong confidence |
|
I should add some context about why I think this is important refactor. When I was looking at the codebase shortly after launch and trying to understand where AI was struggling, one of the things I saw was it really seemed to struggle around having mission critical code sitting inside generated template string. I do think its something we need to change. But at the same time, I do agree with @james-elicx its also important to take our time with. Getting this wrong could really break things in a subtle way |
|
Thanks @james-elicx @southpolesteve — totally agree on the incremental approach. I've opened #345 with snapshot tests for all 5 generators as the first step. Once that merges, I'll follow up with individual extraction PRs (one per generator), so each change is small and the snapshots catch any drift. Plan:
|
|
Closing this in favor of the incremental approach discussed with @james-elicx and @southpolesteve. The all-at-once extraction is too risky given the fast pace of changes on main. Instead, I'll land this as a series of small, focused PRs — each one guarded by the snapshot tests from #345. Incremental plan:
Each PR will be small enough to review confidently. The snapshot tests from step 1 ensure that the generated output remains identical after each extraction — any accidental change to the template strings will show up as a snapshot diff. |
|
Superseded by the incremental extraction series. See the plan above. |
Summary
Extract the 5 template string code generation functions from \index.ts\ and \�pp-dev-server.ts\ into dedicated modules under \packages/vinext/src/entries/.
Changes
New files (\packages/vinext/src/entries/)
Modified files
Pages Router closure conversion
\generateServerEntry\ and \generateClientEntry\ were closures inside \�inext()\ that captured \pagesDir,
extConfig, and \middlewarePath. Converted to explicit function parameters.
Tests
Verification
Closes #253