fix: Add objectParser for ObjectTypeAnnotation (e.g. PARSE_SERVER_AUTH_PROVIDERS) #9912
Conversation
|
I will reformat the title to use the proper commit message syntax. |
parse-github-assistant
bot
changed the title
|
🚀 Thanks for opening this pull request! |
📝 WalkthroughWalkthroughTwo files are modified to add support for parsing object-type configuration values. The build configuration is updated to handle Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes
Possibly related issues
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
resources/buildConfigDefinitions.js (1)
176-242: Consider adding ObjectTypeAnnotation handling for future-proofing.The
parseDefaultValuefunction handles various type annotations but doesn't include a case forObjectTypeAnnotation. While this isn't currently an issue (since theauthoption likely doesn't have a default value), adding this handling would make the function more complete and prevent potential issues if default values are added to object-type fields in the future.Consider adding after line 214:
} else if (t.isBooleanTypeAnnotation(elt)) { literalValue = t.booleanLiteral(parsers.booleanParser(value)); + } else if (t.isObjectTypeAnnotation(elt)) { + const object = parsers.objectParser(value); + const props = Object.keys(object).map(key => { + const val = object[key]; + if (typeof val === 'string') { + return t.objectProperty(t.identifier(key), t.stringLiteral(val)); + } else if (typeof val === 'number') { + return t.objectProperty(t.identifier(key), t.numericLiteral(val)); + } else if (typeof val === 'boolean') { + return t.objectProperty(t.identifier(key), t.booleanLiteral(val)); + } else { + return t.objectProperty(t.identifier(key), t.identifier('undefined')); + } + }); + literalValue = t.objectExpression(props); } else if (t.isGenericTypeAnnotation(elt)) {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
resources/buildConfigDefinitions.js(1 hunks)src/Options/Definitions.js(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-27T09:08:34.252Z
Learnt from: EmpiDev
Repo: parse-community/parse-server PR: 9770
File: src/triggers.js:446-454
Timestamp: 2025-08-27T09:08:34.252Z
Learning: When analyzing function signature changes in Parse Server codebase, verify that call sites are actually incorrect before flagging them. Passing tests are a strong indicator that function calls are already properly aligned with new signatures.
Applied to files:
src/Options/Definitions.js
🧬 Code graph analysis (1)
src/Options/Definitions.js (1)
resources/buildConfigDefinitions.js (1)
parsers(12-12)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (14)
- GitHub Check: Node 20
- GitHub Check: Node 22
- GitHub Check: MongoDB 6, ReplicaSet
- GitHub Check: Node 18
- GitHub Check: MongoDB 8, ReplicaSet
- GitHub Check: PostgreSQL 16, PostGIS 3.5
- GitHub Check: Redis Cache
- GitHub Check: PostgreSQL 18, PostGIS 3.6
- GitHub Check: MongoDB 7, ReplicaSet
- GitHub Check: PostgreSQL 15, PostGIS 3.4
- GitHub Check: PostgreSQL 17, PostGIS 3.5
- GitHub Check: PostgreSQL 15, PostGIS 3.3
- GitHub Check: PostgreSQL 15, PostGIS 3.5
- GitHub Check: Docker Build
🔇 Additional comments (2)
src/Options/Definitions.js (1)
110-115: LGTM! Generated code correctly adds objectParser action.The addition of
action: parsers.objectParserto theauthoption is consistent with other object-type configuration options throughout this file (e.g.,accountLockout,customPages,fileUpload). This ensures that thePARSE_SERVER_AUTH_PROVIDERSenvironment variable is properly parsed as a JSON object rather than treated as a raw string, which directly addresses the PR objective.Note: This is generated code. The actual source changes are in
src/Options/index.jsand the generator scriptresources/buildConfigDefinitions.js.resources/buildConfigDefinitions.js (1)
156-157: LGTM! ObjectTypeAnnotation handling correctly added.The addition of
ObjectTypeAnnotationhandling inmapperForis correct and consistent with the existingAnyTypeAnnotationhandling. This ensures Flow object types (e.g.,?{ [string]: AuthAdapter }) are properly mapped toobjectParser.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## alpha #9912 +/- ##
==========================================
+ Coverage 93.04% 93.06% +0.01%
==========================================
Files 187 187
Lines 15160 15187 +27
Branches 177 177
==========================================
+ Hits 14106 14134 +28
+ Misses 1042 1041 -1
Partials 12 12 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
3 participants
Pull Request
Issue
https://github.com/parse-community/parse-server/issues/9907
The issue occurs because the
mapperForfunction inbuildConfigDefinitions.jsdid not include a specific case for theObjectTypeAnnotationtype in Flow definitions.As a result, configuration options defined as Flow objects - such as:
auth: ?{ [string]: AuthAdapter };were not mapped to the appropriate parser (objectParser) when generating CLI definitions.
This caused environment variables like PARSE_SERVER_AUTH_PROVIDERS to be treated as raw strings instead of being parsed as JSON objects.
In practice, this broke the built-in Microsoft authentication adapter (and any other adapter relying on object-type auth configs), throwing errors such as:
Error: Microsoft options are required.The missing objectParser action in the generated definitions.js file prevented the environment variable from being correctly parsed into an object structure.
Approach
mapperForfunction inbuildConfigDefinitions.jsto handleObjectTypeAnnotationby mapping it to theobjectParser, ensuring object types are parsed correctly.objectParseraction to theauthoption inParseServerOptionsinDefinitions.js, enabling proper parsing of theauthconfiguration as an object.Tasks
Summary by CodeRabbit