Skip to content

Remove Local Build Universal Maker hack#10539

Draft
falahat wants to merge 6 commits into
local_build_fix_envvarsfrom
local_build_remove_um_hack
Draft

Remove Local Build Universal Maker hack#10539
falahat wants to merge 6 commits into
local_build_fix_envvarsfrom
local_build_remove_um_hack

Conversation

@falahat
Copy link
Copy Markdown
Contributor

@falahat falahat commented May 20, 2026

Description

This is no longer required because Universal Maker fixed their bug related to copying files on top of each other.

Also, this cleans up the code for toProcessEnv

Scenarios Tested

Sample Commands

@wiz-9635d3485b
Copy link
Copy Markdown

Wiz Scan Summary

Scanner Findings
Vulnerability Finding Vulnerabilities -
Data Finding Sensitive Data -
Secret Finding Secrets -
IaC Misconfiguration IaC Misconfigurations -
SAST Finding SAST Findings 1 Medium
Software Management Finding Software Management Findings -
Total 1 Medium

View scan details in Wiz

To detect these findings earlier in the dev lifecycle, try using Wiz Code VS Code Extension.

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request updates the build output directory to .apphosting and removes redundant file-moving logic that was previously used to handle output safely. Additionally, the toProcessEnv function has been refactored to improve readability and type safety, aligning with the repository's style guide by reducing nesting and streamlining the environment variable resolution process.

Comment on lines +255 to 270
async function toProcessEnv(projectId: string, env: EnvMap): Promise<Record<string, string>> {
const buildVars = Object.entries(env).filter(([, value]) => {
return !value.availability || value.availability.includes("BUILD");
});

if (value.secret) {
const resolvedValue = await loadSecret(projectId, value.secret);
return [key, resolvedValue];
} else {
return [key, value.value || ""];
}
const resolvedEntries = await Promise.all(
buildVars.map(async ([key, value]) => {
const resolvedValue = value.secret
? await loadSecret(projectId, value.secret)
: value.value || "";
return [key, resolvedValue];
}),
);

const filteredEntries = entries.filter((entry): entry is [string, string] => entry !== null);
return Object.fromEntries(filteredEntries) as NodeJS.ProcessEnv;
return Object.fromEntries(resolvedEntries) as Record<string, string>;
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

The refactoring of toProcessEnv significantly improves readability and adheres to the repository's best practice of reducing nesting. By filtering the environment variables before mapping them to asynchronous secret resolution, the code is more efficient and easier to follow. Additionally, changing the return type to Record<string, string> provides better type safety and aligns with the expectations of runUniversalMaker.

References
  1. Reduce nesting as much as possible. Code should avoid unnecessarily deep nesting or long periods of nesting. Handle edge cases early and exit or fold them into the general case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants