Repository navigation
refactor(functions): introduce 3-stage kit install pipeline - #11366
Conversation
There was a problem hiding this comment.
Code Review
This pull request refactors the function kits installation logic by extracting source-specific handling into a modular adapter pattern under src/functions/kits/sources/. It introduces KitSourceAdapter implementations for both local directory and NPM package sources, simplifying install.ts into a clean three-stage orchestration flow. Feedback on the changes highlights several potential TypeError vulnerabilities where fs.readJson could return null and cause crashes on property access. Additionally, multiple functions accepting more than three arguments violate the repository style guide and should be refactored to accept a single options object.
e1d8f63 to
7b78ce8
Compare
7b78ce8 to
8faa839
Compare
8faa839 to
8dc4ddb
Compare
8dc4ddb to
d10a1b9
Compare
618d7bf to
4950a70
Compare
59020b5 to
e95c030
Compare
e95c030 to
de5819c
Compare
de5819c to
0b41d76
Compare
ajperel
left a comment
There was a problem hiding this comment.
I think we pushed a bit too much into the different stage methods and shouldn't need action and isNewKit in them, but overall, I think this is a good improvement. Thanks for taking the time to do it.
0b41d76 to
9ba8139
Compare
9ba8139 to
8fb4285
Compare
8fb4285 to
903fd03
Compare
dcc2b1f to
36e8e4e
Compare
ajperel
left a comment
There was a problem hiding this comment.
I was probably a bit stingy not just approving with my last round of comments so just doing it now. Haven't re-reviewed. I trust you to address the feedback.
36e8e4e to
807beba
Compare
… and layer boundaries
807beba to
970e27f
Compare
Description
Refactors Function Kits installation (
src/functions/kits/install.tsandsrc/functions/kits/install.spec.ts) following #11370 to consumeKitSourceAdapter(createNpmPackageAdapter/createDirectoryAdapter) and separate kit-level setup, instance resolution, and project configuration, with no user-facing behavior changes:install.ts): SimplifiesinstallKitOrInstanceinto three source-agnostic stages:ensureKit: Resolves an existing kit viaadapter.findExistingKitor validates, prepares (adapter.prepareSource), and stages the new kit in-memory (write: false).resolveOrCreateInstance: Creates a new kit instance (addInstanceToKitConfig, which persists both the staged kit and instance tofirebase.json) or selects an existing unconfigured instance for the active project (promptExistingInstanceForProject).configureInstanceForProject: Seeds.env.<projectId>, discovers build parameters (discoverKitBuild), and prompts for values (promptAndWriteKitParams).printKitFirstDeployReportrun ininstallKitOrInstanceafter pipeline completion, branching directly onisNewKitandisNewInstance.KitSource,resolvePackageSource,resolveDirectorySource,findExistingKit,scaffoldKit, andaddKitInstanceOrConfigureProjectin favor of the unifiedKitSourceAdapterpipeline.install.ts(getUnconfiguredInstancesForProject,promptExistingInstanceForProject,discoverKitBuild,printKitFirstDeployReport,promptAndWriteKitParams) into explicit*Optionsinterfaces.Scenarios Tested
src/functions/kits/**/*.spec.ts,src/commands/functions-kits-install.spec.ts, andsrc/extensions/migrate.spec.ts.--package) and from a local directory (--directory).Sample Commands
No CLI flags or commands changed: