Skip to content

Conversation

@Tobbe
Copy link
Member

@Tobbe Tobbe commented Jan 14, 2026

Code formatting was silently skipped when using the generators if a project had a .mjs prettier config file. This would lead to code full of red squiggles because of formatting "errors". This PR updates the formatting code to look for both .cjs and .mjs config files.

@github-actions github-actions bot added this to the next-release milestone Jan 14, 2026
@netlify
Copy link

netlify bot commented Jan 14, 2026

Deploy Preview for cedarjs canceled.

Name Link
🔨 Latest commit 4fdb4f0
🔍 Latest deploy log https://app.netlify.com/projects/cedarjs/deploys/69670c596b843e0008959291

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Jan 14, 2026

Greptile Summary

This PR adds support for prettier.config.mjs configuration files as a fallback when prettier.config.cjs doesn't exist. While the feature itself is implemented correctly in the TypeScript helpers file, there is a critical breaking change in the JavaScript CLI file where the generateTemplate function was accidentally changed from async to non-async, despite still returning a Promise from prettify(). This breaks all downstream code that expects the function to be awaitable.

Confidence Score: 1/5

  • This PR has a critical bug that breaks the public API and will cause runtime failures in all callers of generateTemplate.
  • The PR introduces a critical breaking change by removing the async keyword from generateTemplate while the function still returns a Promise. All callers use await generateTemplate(...), expecting an async function. This change will cause all callers to receive a Promise instead of a string, breaking the generate command functionality. While the prettier config fallback feature is valid, this regression makes the PR unsafe to merge.
  • packages/cli/src/lib/index.js - The generateTemplate function must be restored to async to maintain API compatibility.

Important Files Changed

Filename Overview
packages/cli/src/lib/index.js Added support for prettier.config.mjs in getPrettierOptions function. However, the generateTemplate function was changed from async to non-async while still returning a Promise from prettify(), breaking all callers expecting the function to be awaitable.
packages/cli-helpers/src/lib/index.ts Added support for prettier.config.mjs fallback in the getPrettierOptions function. Changes correctly maintain the async behavior and error handling. Documentation updated to reference Cedar instead of Redwood.

Sequence Diagram

sequenceDiagram
    participant Caller as Generate Command
    participant Generate as generateTemplate()
    participant Prettify as prettify()
    participant Prettier as getPrettierOptions()
    participant FS as fs/import

    Caller->>Generate: call generateTemplate(file, params)
    Note over Generate: Now returns Promise<br/>(was async)
    Generate->>Prettify: return prettify() [Promise]
    Caller->>Caller: ERROR: Expected string,<br/>got Promise!
    
    alt Original Async Flow
        Caller->>Generate: await generateTemplate()
        Generate->>Prettify: await prettify()
        Prettify->>Prettier: await getPrettierOptions()
        Prettier->>FS: fs.existsSync(cjsPath)
        Prettier->>FS: fs.existsSync(mjsPath)
        FS-->>Prettier: return path
        Prettier->>FS: import(prettierConfigPath)
        FS-->>Prettier: return config
        Prettier-->>Prettify: return options
        Prettify->>FS: format(content, options)
        FS-->>Prettify: return formatted
        Prettify-->>Generate: return formatted
        Generate-->>Caller: return formatted string
    end
Loading

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

2 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

Comment on lines +61 to 70
export const generateTemplate = (templateFilename, { name, ...rest }) => {
try {
const templateFn = template(readFile(templateFilename).toString())

const renderedTemplate = templateFn({
name,
...nameVariants(name),
...rest,
})

return prettify(templateFilename, renderedTemplate)
Copy link
Contributor

Choose a reason for hiding this comment

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

logic: The function signature was changed from async to non-async, but prettify returns a Promise. This breaks the function's return value - callers expecting a formatted string will now receive a Promise instead. This is a breaking change that will cause runtime errors in any code calling generateTemplate and expecting a string result.

All callers of generateTemplate need to handle it as an async function.

Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/cli/src/lib/index.js
Line: 61:70

Comment:
**logic:** The function signature was changed from `async` to non-async, but `prettify` returns a Promise. This breaks the function's return value - callers expecting a formatted string will now receive a Promise instead. This is a breaking change that will cause runtime errors in any code calling `generateTemplate` and expecting a string result.

All callers of `generateTemplate` need to handle it as an async function.

How can I resolve this? If you propose a fix, please make it concise.

@nx-cloud
Copy link

nx-cloud bot commented Jan 14, 2026

🤖 Nx Cloud AI Fix

Ensure the fix-ci command is configured to always run in your CI pipeline to get automatic fixes in future runs. For more information, please see https://nx.dev/ci/features/self-healing-ci


View your CI Pipeline Execution ↗ for commit 4fdb4f0

Command Status Duration Result
nx run-many -t test --minWorkers=1 --maxWorkers=4 ✅ Succeeded 1m 54s View ↗
nx run-many -t build:pack --exclude create-ceda... ✅ Succeeded 22s View ↗
nx run-many -t build ✅ Succeeded 57s View ↗
nx run-many -t test:types ✅ Succeeded 9s View ↗

☁️ Nx Cloud last updated this comment at 2026-01-14 03:42:04 UTC

@nx-cloud
Copy link

nx-cloud bot commented Jan 14, 2026

🤖 Nx Cloud AI Fix

Ensure the fix-ci command is configured to always run in your CI pipeline to get automatic fixes in future runs. For more information, please see https://nx.dev/ci/features/self-healing-ci


View your CI Pipeline Execution ↗ for commit 4fdb4f0

Command Status Duration Result
nx run-many -t build:pack --exclude create-ceda... ✅ Succeeded 14s View ↗
nx run-many -t build ✅ Succeeded 40s View ↗

☁️ Nx Cloud last updated this comment at 2026-01-14 03:27:52 UTC

@nx-cloud
Copy link

nx-cloud bot commented Jan 14, 2026

🤖 Nx Cloud AI Fix

Ensure the fix-ci command is configured to always run in your CI pipeline to get automatic fixes in future runs. For more information, please see https://nx.dev/ci/features/self-healing-ci


View your CI Pipeline Execution ↗ for commit 4fdb4f0

Command Status Duration Result
nx run-many -t build ✅ Succeeded 52s View ↗
nx run-many -t build:pack --exclude create-ceda... ✅ Succeeded 18s View ↗
nx run-many -t test --minWorkers=1 --maxWorkers=4 ✅ Succeeded 1m 35s View ↗
nx run-many -t test:types ✅ Succeeded 6s View ↗

☁️ Nx Cloud last updated this comment at 2026-01-14 03:30:51 UTC

@Tobbe
Copy link
Member Author

Tobbe commented Jan 14, 2026

@greptileai you said:

The PR introduces a critical breaking change by removing the async keyword from generateTemplate while the function still returns a Promise. All callers use await generateTemplate(...), expecting an async function. This change will cause all callers to receive a Promise instead of a string, breaking the generate command functionality. While the prettier config fallback feature is valid, this regression makes the PR unsafe to merge.

If all callers already do await, isn't the code still safe?

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

You're absolutely right! I made an error in my analysis. If all callers are already using await generateTemplate(...), then removing the async keyword is actually safe.

Here's why:

  • generateTemplate still returns a Promise (from the prettify() call)
  • await works on any Promise, whether the function is declared async or not
  • The callers will still get the resolved value (string) instead of a Promise object

The async keyword was unnecessary since the function doesn't use await internally - it just returns the Promise directly from prettify(). Removing it is actually a minor improvement, not a breaking change.

My apologies for the confusion in my original review. The PR is safe to merge as-is. The prettier config fallback feature works correctly and the generateTemplate change doesn't introduce any breaking functionality.

@Tobbe Tobbe merged commit 2fcbc56 into main Jan 14, 2026
42 checks passed
@Tobbe Tobbe deleted the tobbe-feat-prettify-mjs branch January 14, 2026 03:52
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.

1 participant