refactor: extract OG asset plugins into src/plugins/og-assets.ts#706
refactor: extract OG asset plugins into src/plugins/og-assets.ts#706james-elicx merged 1 commit intomainfrom
Conversation
Move vinext:og-inline-fetch-assets and vinext:og-assets out of the monolithic index.ts into a focused file (src/plugins/og-assets.ts). - createOgInlineFetchAssetsPlugin() factory: owns the per-build cache (Map<string, string>) and isBuild flag as closure state, removing the two module-level variables that leaked into the outer vinext() factory closure. - ogAssetsPlugin constant: the writeBundle-only plugin has no mutable state, so it exports directly as a plain plugin object. index.ts now imports and delegates to these, removing ~190 lines of inline plugin code from the main plugin file.
commit: |
|
/bigbonk review |
|
![ask-bonk[bot]](https://avatars.githubusercontent.com/in/2454580?s=60&v=4){kind=small}
There was a problem hiding this comment.
$(cat <<'EOF'
Clean extraction. The code is a 1:1 move — logic, comments, and variable names all match the original. The factory pattern for createOgInlineFetchAssetsPlugin correctly internalizes the cache and isBuild state that was previously leaking into the vinext() closure scope, and the import placement in index.ts is consistent with the other plugins/* imports.
One minor observation (non-blocking): ogAssetsPlugin is exported as a singleton object rather than a factory. Since this plugin is stateless (no mutable fields, only a writeBundle hook that reads from disk), sharing a single object reference across multiple vinext() calls is fine. If mutable state were ever added to it, it would need the same factory treatment — but as-is, it is correct.
LGTM.
EOF
)
|
Approved PR #706. The extraction is clean — it's a faithful 1:1 move of the two OG asset plugins from the monolithic Key observations:
|
1 participant
Summary
vinext:og-inline-fetch-assetsandvinext:og-assetsout of the monolithicindex.tsintopackages/vinext/src/plugins/og-assets.tscreateOgInlineFetchAssetsPlugin()factory: the per-buildMap<string, string>cache andisBuildflag become closure state inside the factory, removing two module-level variables that previously leaked into the outervinext()closureogAssetsPluginconstant: thewriteBundle-only plugin has no mutable state, so it exports directly as a plain plugin objectindex.tsimports and delegates to these, removing ~190 lines of inline plugin codeTesting
vp checkpasses on both changed files (no type errors, no lint errors, no format issues)pnpm test tests/features.test.ts— 264/264 pass