Skip to content

use top-level await instead of async IIFE in graphql-codegen.ts#3398

Merged
arkid15r merged 3 commits intoOWASP:mainfrom
sameersharmadev:top-level-async
Jan 21, 2026
Merged

use top-level await instead of async IIFE in graphql-codegen.ts#3398
arkid15r merged 3 commits intoOWASP:mainfrom
sameersharmadev:top-level-async

Conversation

@sameersharmadev
Copy link
Contributor

@sameersharmadev sameersharmadev commented Jan 18, 2026

Proposed change

Resolves #3087
Replaces the async IIFE with top-level await in frontend/graphql-codegen.ts for improved readability and maintainability.
Verified by make graphql-codegen
image

Please take a look at this suggestion before merging

Checklist

  • Required: I followed the contributing workflow
  • Required: I verified that my code works as intended and resolves the issue as described
  • Required: I ran make check-test locally: all warnings addressed, tests passed
  • I used AI for code, documentation, tests, or communication related to this PR

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 18, 2026

Summary by CodeRabbit

  • Refactor
    • Restructured internal configuration handling with improved error reporting during build processes to provide faster feedback on configuration failures.

✏️ Tip: You can customize this high-level summary in your review settings.

Walkthrough

Replaces an async IIFE export in frontend/graphql-codegen.ts with a top-level CodegenConfig constant and default export; fetch CSRF handling now logs failures and calls process.exit(1); configuration shape reorganized into a single named config object with explicit plugin/preset blocks.

Changes

Cohort / File(s) Summary
GraphQL Codegen Configuration Refactor
frontend/graphql-codegen.ts
Replaced exported async IIFE with a top-level CodegenConfig constant and export default config; moved documents/generates/schema into the consolidated config; changed CSRF fetch failure and non-ok handling to log and process.exit(1); reorganized plugin, preset, and presetConfig placement; preserved flags like ignoreNoDocuments and overwrite.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Suggested reviewers

  • arkid15r
  • kasya
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: replacing an async IIFE with top-level await in graphql-codegen.ts, which is the primary objective of this PR.
Description check ✅ Passed The description clearly relates to the changeset, explaining the motivation to use top-level await instead of async IIFE, referencing the resolved issue #3087, and confirming verification through testing.
Linked Issues check ✅ Passed The code changes successfully implement the primary objective from issue #3087 by converting from async IIFE pattern to top-level await in frontend/graphql-codegen.ts for improved readability and alignment with modern ES module standards.
Out of Scope Changes check ✅ Passed The changes are entirely within scope, focusing solely on refactoring the async IIFE pattern to use top-level await while preserving all configuration behavior and error handling logic as required by issue #3087.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Fix all issues with AI agents
In `@frontend/graphql-codegen.ts`:
- Around line 5-15: Replace the top-level await fetch with an exported async
function: wrap the CSRF fetch logic inside "export default async function()"
(e.g., perform "const response = await fetch(`${PUBLIC_API_URL}/csrf/`, {
method: 'GET' })" inside that function), handle errors inside the function (log
the error and throw or return a rejected Promise instead of calling
process.exit), and return whatever configuration/value the GraphQL Codegen
consumer expects; update any references to response to use the function-local
variable (this file's top-level fetch and process.exit usage should be removed
and replaced by the async export).
- Around line 11-19: The catch block currently logs and calls process.exit(1)
for network errors while the response.ok branch throws an Error, causing
inconsistent behaviour; change the catch to accept the error (e.g., catch (err))
and rethrow a normalized Error (include the original err message) instead of
calling process.exit, so both network and HTTP failures throw an Error (keep the
existing throw in the response.ok branch) and let the top-level handler decide
exit behavior; update the catch to reference the same failure message format
used in the response.ok check and include the original error details.

coderabbitai[bot]
coderabbitai bot previously approved these changes Jan 18, 2026
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 18, 2026

✅ Actions performed

Full review triggered.

@sameersharmadev
Copy link
Contributor Author

@coderabbitai full review

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 18, 2026

✅ Actions performed

Full review triggered.

Copy link
Collaborator

@rudransh-shrivastava rudransh-shrivastava left a comment

Choose a reason for hiding this comment

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

Hey, nice work!
Just a small change:

Copy link
Collaborator

@rudransh-shrivastava rudransh-shrivastava Jan 19, 2026

Choose a reason for hiding this comment

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

Let's keep it as a .ts file. I think removing the IIFE should resolve the Sonar Qube issue.

Copy link
Contributor Author

@sameersharmadev sameersharmadev Jan 19, 2026

Choose a reason for hiding this comment

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

@rudransh-shrivastava Okay, I've reverted back to .ts now 👍 , ready for review, but please take the look at Coderabbit suggestion, it seems to advise aaginst using top level await

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 19, 2026

✅ Actions performed

Full review triggered.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@frontend/graphql-codegen.ts`:
- Around line 5-21: Replace the top-level await fetch logic with an exported
async factory function: wrap the existing fetch/response handling (the try/catch
around fetch and the response.ok check that produces csrfToken) inside export
default async function() and return the GraphQL Codegen config object from that
function (so the csrfToken variable is scoped inside the function and used to
build the returned config). Ensure you preserve the same error handling around
response and use (await response.json()).csrftoken to set csrfToken before
returning the config.

@sonarqubecloud
Copy link

Copy link
Collaborator

@rudransh-shrivastava rudransh-shrivastava left a comment

Choose a reason for hiding this comment

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

Looks good to me!

Copy link
Collaborator

@kasya kasya left a comment

Choose a reason for hiding this comment

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

@sameersharmadev nice work! 👍🏼

@arkid15r arkid15r added this pull request to the merge queue Jan 21, 2026
Merged via the queue into OWASP:main with commit 50b669b Jan 21, 2026
29 of 30 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Prefer top-level await instead of async IIFE (Sonar S7785)

4 participants

Comments