Skip to content

refactor(cf-deploy-config-writer): consolidate disk-write template rendering (#4458)#4462

Open
longieirl wants to merge 10 commits intomainfrom
refactor/4458-cf-deploy-config-template-rendering
Open

refactor(cf-deploy-config-writer): consolidate disk-write template rendering (#4458)#4462
longieirl wants to merge 10 commits intomainfrom
refactor/4458-cf-deploy-config-template-rendering

Conversation

@longieirl
Copy link
Copy Markdown
Contributor

Summary

  • Introduces src/mta-config/template-renderer.ts with a single renderTemplateToDisk() function
  • Removes duplicated readFileSync + render + writeFileSync pattern from mta.ts and index.ts
  • Centralises template path resolution through the existing getTemplatePath() utility
  • Makes the intentional mem-fs bypass explicit and documented in one place

Related

Closes part of #4458

Test plan

  • 3 new unit tests for renderTemplateToDisk (100% coverage)
  • All 72 existing tests pass unchanged
  • No public API or mta.yaml output changes

…ndering

Extract readFileSync+render+writeFileSync into renderTemplateToDisk() in
template-renderer.ts. Update createMTA, createCAPMTAAppFrontend, and
addMtaExtensionConfig to use it. Replace the hardcoded __dirname-relative
path in mta.ts with getTemplatePath() for consistent resolution.
@changeset-bot
Copy link
Copy Markdown

changeset-bot bot commented Mar 25, 2026

🦋 Changeset detected

Latest commit: cd487bc

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 3 packages
Name Type
@sap-ux/cf-deploy-config-writer Patch
@sap-ux/cf-deploy-config-sub-generator Patch
@sap-ux/deploy-config-sub-generator Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

…late render

Template is read from a known internal path via getTemplatePath(), not from
user input — NOSONAR suppression is warranted.
@longieirl longieirl requested a review from heimwege March 26, 2026 10:17
mikicvi-SAP
mikicvi-SAP previously approved these changes Apr 3, 2026
Copy link
Copy Markdown
Contributor

@mikicvi-SAP mikicvi-SAP left a comment

Choose a reason for hiding this comment

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

Thanks @longieirl

  • Changes look good and easy to follow
  • Test covers the change and well doc'd
  • Changeset ✅

kjose90
kjose90 previously approved these changes Apr 10, 2026
Copy link
Copy Markdown
Member

@kjose90 kjose90 left a comment

Choose a reason for hiding this comment

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

LGTM! The cleanup looks good - Approved.
Changeset, tests ✅

@longieirl longieirl self-assigned this Apr 13, 2026
Copy link
Copy Markdown
Contributor

@heimwege heimwege left a comment

Choose a reason for hiding this comment

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

See comments


refactor: consolidate disk-write template rendering into a single module

Introduced `src/mta-config/template-renderer.ts` with `renderTemplateToDisk()`,
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.

This might be a bit too much information for the changeset

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.

Updated. Thanks.

*/
export function renderTemplateToDisk(templateName: string, outputPath: string, data: Record<string, unknown>): void {
const template = readFileSync(getTemplatePath(templateName), 'utf-8');
writeFileSync(outputPath, render(template, data)); // NOSONAR - template content is read from a known internal path resolved via getTemplatePath(), not from user input
Copy link
Copy Markdown
Contributor

@heimwege heimwege Apr 13, 2026

Choose a reason for hiding this comment

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

I have to admit I don't know which option is better. Add NOSONAR directly in the code or mark the sonar finding as accept in sonar 🤷🏼‍♂️
Maybe we should decide on one and harmonize across the monorepo/adjust agents.md accordingly.

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.

Updated. Thanks. I agree, I think its better to have the sonar finding manually accepted.

- Remove NOSONAR comment from template-renderer.ts (to be accepted in SonarCloud UI)
- Trim changeset message to a single concise line
@longieirl longieirl dismissed stale reviews from mikicvi-SAP and kjose90 via 142a7ec April 14, 2026 09:24
@sonarqubecloud
Copy link
Copy Markdown

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.

5 participants