Increase heap size in validation workflow and fix type errors

[danielnaab]

No description provided.

[changeset-bot]

⚠️ No Changeset found

Latest commit: 27f1c97

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

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

[claude]

PR Review: Increase heap size in validation workflow + Import path standardization

Overview

This PR makes two main changes:

1. CI Fix: Increases Node heap size to 4GB for Terraform CDK workflow step
2. Code Quality: Standardizes import paths to use explicit .js extensions throughout the packages/forms codebase

✅ Positives

CI/Workflow Changes:

• The heap size increase (NODE_OPTIONS: --max-old-space-size=4096) is a reasonable fix for CI memory issues
• Properly scoped to only the problematic step (Terraform CDK initialization)

Import Path Standardization:

• All import paths now use explicit .js extensions, aligning with ES module best practices
• Consistent application across 22 files shows thoroughness
• This improves TypeScript/Node.js ESM compatibility

Code Improvements:

• packages/forms/src/pattern.ts:251-253: Excellent addition! The null check for root pattern prevents potential runtime errors
• packages/forms/src/repository/delete-form.ts:42: Removed unnecessary type annotation on catch parameter (cleaner)
• packages/forms/src/patterns/repeater/submit.ts:18: Removed unnecessary type annotation (TypeScript can infer)

Package.json Additions:

• Added clean scripts to e2e/ and infra/aws-cdk/ packages
• Enhanced clean:lib script in design package for more thorough cleanup

🔍 Observations & Suggestions

1. Missing Root Cause Analysis

• The PR title mentions "attempt to fix CI failure" but doesn't explain:
  • What error was occurring?
  • Why does Terraform CDK need 4GB heap?
  • Is this a temporary workaround or permanent solution?
• Recommendation: Add details to PR description about the underlying issue

2. Scope Mixing

• This PR combines two unrelated changes:
  • CI memory fix (infrastructure)
  • Import path refactoring (code quality)
• Recommendation: Consider splitting these into separate PRs for clearer change tracking

3. Import Path Convention

• While .js extensions are technically correct for ESM, note that the codebase uses conditional exports for development:
  • Development: resolves to .ts source files
  • Production: resolves to .js built files
• The .js extension in imports is correct per Node.js ESM spec (imports reference the runtime output, not source)
• Question: Are there corresponding changes needed in tsconfig.json or build configurations?

4. Removed Dependency

• packages/forms/src/pattern.ts removed import set from 'set-value' but I don't see its usage removed in the diff
• Recommendation: Verify this import was actually unused, and if so, remove from package.json dependencies

5. Minor Type Safety

// packages/forms/src/patterns/repeater/submit.ts:69
const updatedRepeaterData = repeaterPatternData.filter(
  (_: any, index: number) => index !== indexToDelete
);

• Still uses _: any parameter
• Suggestion: Use (_: unknown, index: number) for slightly better type safety

🧪 Testing Considerations

• Test Coverage: No test file changes - existing tests should validate the import path changes don't break functionality
• Recommendation: Run full test suite locally to ensure all import paths resolve correctly in both dev and production modes
• CI Validation: The heap size increase should be monitored in CI to confirm it resolves the issue

🔒 Security

• No security concerns identified
• The heap size increase is reasonable and doesn't expose security risks

⚡ Performance

• Heap size increase may slightly slow CI, but 4GB is reasonable for Terraform CDK operations
• Import path changes are neutral for performance (transpiler handles both syntaxes)

📊 Overall Assessment

Quality: ✅ Good
Risk Level: Low
Recommendation: Approve with minor suggestions

The code changes are clean and improve standards compliance. The main suggestion is to document the CI issue in the PR description and consider separating the two independent changes for better git history.

---

🤖 Generated with Claude Code