Skip to content

fix: Add objectParser for ObjectTypeAnnotation (e.g. PARSE_SERVER_AUTH_PROVIDERS) #9912

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 6 commits into
base: alpha
Choose a base branch
from
+159 −0
Open

fix: Add objectParser for ObjectTypeAnnotation (e.g. PARSE_SERVER_AUTH_PROVIDERS) #9912

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
996205b
fix: Add objectParser action for ParseServerOptions in Definitions.js
Giacomo117 Nov 7, 2025
81b88f1
Merge branch 'parse-community:alpha' into fix/auth-env-bug
mattia1208 Nov 12, 2025
d794856
Merge branch 'alpha' of https://github.com/E38-Software/parse-server …
Giacomo117 Nov 12, 2025
c72fff6
Merge branch 'fix/auth-env-bug' of https://github.com/E38-Software/pa…
Giacomo117 Nov 12, 2025
69875a2
test: Add unit tests for mapperFor function in buildConfigDefinitions
Giacomo117 Nov 12, 2025
8224a93
feat: Export mapperFor function for testing purposes
Giacomo117 Nov 12, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions resources/buildConfigDefinitions.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,8 @@ function mapperFor(elt, t) {
return wrap(t.identifier('objectParser'));
} else if (t.isBooleanTypeAnnotation(elt)) {
return wrap(t.identifier('booleanParser'));
} else if (t.isObjectTypeAnnotation(elt)) {
return wrap(t.identifier('objectParser'));
} else if (t.isGenericTypeAnnotation(elt)) {
const type = elt.typeAnnotation.id.name;
if (type == 'Adapter') {
Expand Down Expand Up @@ -381,3 +383,6 @@ const res = babel.transformFileSync('./src/Options/index.js', {
});
require('fs').writeFileSync('./src/Options/Definitions.js', res.code + '\n');
require('fs').writeFileSync('./src/Options/docs.js', docs);

// Export mapperFor for testing
module.exports = { mapperFor };
Comment on lines +386 to +388
Copy link

@coderabbitai coderabbitai bot Nov 12, 2025

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major

🧩 Analysis chain

Verify that tests handle file system side effects.

Exporting mapperFor enables testing, which is good. However, when this module is required (e.g., by tests), it will execute the babel transformation at lines 378-385, which writes to Definitions.js and docs.js. Ensure that your test setup either:

  • Mocks the file system operations, or
  • Runs in an isolated environment where these writes are acceptable, or
  • Uses a test-specific entry point that only exports the function without running the build

🏁 Script executed:

#!/bin/bash
# Description: Check if tests mock fs operations or handle build side effects

# Look for test files that import buildConfigDefinitions
rg -n --type=js -A 5 "require.*buildConfigDefinitions|import.*buildConfigDefinitions"

# Check if tests mock fs.writeFileSync
rg -n --type=js -B 3 -A 3 "writeFileSync.*mock|mock.*writeFileSync|jest\.mock.*fs"

Length of output: 471

🏁 Script executed:

#!/bin/bash
# Get full context of buildConfigDefinitions.js around the export and transformation

echo "=== buildConfigDefinitions.js lines 370-390 ==="
sed -n '370,390p' resources/buildConfigDefinitions.js

echo ""
echo "=== Check if transformation is wrapped in conditionals ==="
sed -n '375,385p' resources/buildConfigDefinitions.js

echo ""
echo "=== Check test file setup/teardown ==="
head -40 spec/buildConfigDefinitions.spec.js

Length of output: 2399

🏁 Script executed:

#!/bin/bash
# Check for NODE_ENV or test environment conditions in buildConfigDefinitions.js

rg -n "process\.env|NODE_ENV|process\.argv" resources/buildConfigDefinitions.js

# Check if there's a test helper or setup file
fd -e "js" -path "*/spec/*" -name "*setup*|*helper*|*config*"

Length of output: 168

🏁 Script executed:

#!/bin/bash
# Check if Jest is configured with setupFilesAfterEnv or similar that might mock fs

rg -n "jest|setupFiles|moduleNameMapper" jest.config.js package.json 2>/dev/null || echo "Config files not found in root, checking elsewhere"

# Look for jest configuration
fd -e "js" -o -e "json" -name "jest.config*" -o -name "package.json" -type f | head -5

Length of output: 167

🏁 Script executed:

#!/bin/bash
# Check if generated files are ignored or tracked

echo "=== Check .gitignore for src/Options/*.js ==="
grep -n "Options\|Definitions\|docs" .gitignore 2>/dev/null || echo "Pattern not found in .gitignore"

echo ""
echo "=== Check if generated files exist and their status ==="
ls -la src/Options/Definitions.js src/Options/docs.js 2>/dev/null || echo "Files not found (may not exist yet)"

echo ""
echo "=== Check if these files are versioned in git ==="
git ls-files src/Options/Definitions.js src/Options/docs.js 2>/dev/null || echo "Files not tracked"

Length of output: 372

🏁 Script executed:

#!/bin/bash
# Check if there's jest config at root or package.json jest config
cat package.json | grep -A 20 '"jest"' 2>/dev/null || echo "No jest config in package.json"

# Also try to find jest config files
find . -maxdepth 2 -name "jest.config.*" 2>/dev/null

Length of output: 30

🏁 Script executed:

#!/bin/bash
# Check if buildConfigDefinitions.js is only executed during build, not by tests
# Look for where buildConfigDefinitions.js is actually imported/used

rg -n "buildConfigDefinitions" --type=js -B 2 -A 2 | head -50

Length of output: 1499

Refactor to prevent file system writes during test import.

The side effect concern is confirmed: the transformation at lines 378-385 executes unconditionally on import, writing to versioned files src/Options/Definitions.js and src/Options/docs.js every time tests run.

Use one of these approaches:

  • Add if (require.main === module) guard around the transformation, so it only runs when executed directly
  • Add NODE_ENV === 'test' check to skip transformation during testing
  • Mock fs.writeFileSync in test setup
  • Create a separate test export that only exports mapperFor without running the build
🤖 Prompt for AI Agents 
In resources/buildConfigDefinitions.js around lines 378–388, the unconditional
transformation/writes run on module import (causing file system writes during
tests); guard the transformation so it only runs when executed directly (e.g.,
wrap the write/transform block in if (require.main === module) or skip when
process.env.NODE_ENV === 'test'), leaving module.exports = { mapperFor } intact
so tests can import mapperFor without triggering any file writes.

153 changes: 153 additions & 0 deletions spec/buildConfigDefinitions.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,153 @@
const t = require('@babel/types');
const { mapperFor } = require('../resources/buildConfigDefinitions');

describe('buildConfigDefinitions', () => {
describe('mapperFor', () => {
it('should return objectParser for ObjectTypeAnnotation', () => {
const mockElement = {
type: 'ObjectTypeAnnotation',
};

const result = mapperFor(mockElement, t);

expect(t.isMemberExpression(result)).toBe(true);
expect(result.object.name).toBe('parsers');
expect(result.property.name).toBe('objectParser');
});

it('should return objectParser for AnyTypeAnnotation', () => {
const mockElement = {
type: 'AnyTypeAnnotation',
};

const result = mapperFor(mockElement, t);

expect(t.isMemberExpression(result)).toBe(true);
expect(result.object.name).toBe('parsers');
expect(result.property.name).toBe('objectParser');
});

it('should return arrayParser for ArrayTypeAnnotation', () => {
const mockElement = {
type: 'ArrayTypeAnnotation',
};

const result = mapperFor(mockElement, t);

expect(t.isMemberExpression(result)).toBe(true);
expect(result.object.name).toBe('parsers');
expect(result.property.name).toBe('arrayParser');
});

it('should return booleanParser for BooleanTypeAnnotation', () => {
const mockElement = {
type: 'BooleanTypeAnnotation',
};

const result = mapperFor(mockElement, t);

expect(t.isMemberExpression(result)).toBe(true);
expect(result.object.name).toBe('parsers');
expect(result.property.name).toBe('booleanParser');
});

it('should return numberParser call expression for NumberTypeAnnotation', () => {
const mockElement = {
type: 'NumberTypeAnnotation',
name: 'testNumber',
};

const result = mapperFor(mockElement, t);

expect(t.isCallExpression(result)).toBe(true);
expect(result.callee.property.name).toBe('numberParser');
expect(result.arguments[0].value).toBe('testNumber');
});

it('should return moduleOrObjectParser for Adapter GenericTypeAnnotation', () => {
const mockElement = {
type: 'GenericTypeAnnotation',
typeAnnotation: {
id: {
name: 'Adapter',
},
},
};

const result = mapperFor(mockElement, t);

expect(t.isMemberExpression(result)).toBe(true);
expect(result.object.name).toBe('parsers');
expect(result.property.name).toBe('moduleOrObjectParser');
});

it('should return numberOrBooleanParser for NumberOrBoolean GenericTypeAnnotation', () => {
const mockElement = {
type: 'GenericTypeAnnotation',
typeAnnotation: {
id: {
name: 'NumberOrBoolean',
},
},
};

const result = mapperFor(mockElement, t);

expect(t.isMemberExpression(result)).toBe(true);
expect(result.object.name).toBe('parsers');
expect(result.property.name).toBe('numberOrBooleanParser');
});

it('should return numberOrStringParser call expression for NumberOrString GenericTypeAnnotation', () => {
const mockElement = {
type: 'GenericTypeAnnotation',
name: 'testString',
typeAnnotation: {
id: {
name: 'NumberOrString',
},
},
};

const result = mapperFor(mockElement, t);

expect(t.isCallExpression(result)).toBe(true);
expect(result.callee.property.name).toBe('numberOrStringParser');
expect(result.arguments[0].value).toBe('testString');
});

it('should return arrayParser for StringOrStringArray GenericTypeAnnotation', () => {
const mockElement = {
type: 'GenericTypeAnnotation',
typeAnnotation: {
id: {
name: 'StringOrStringArray',
},
},
};

const result = mapperFor(mockElement, t);

expect(t.isMemberExpression(result)).toBe(true);
expect(result.object.name).toBe('parsers');
expect(result.property.name).toBe('arrayParser');
});

it('should return objectParser for unknown GenericTypeAnnotation', () => {
const mockElement = {
type: 'GenericTypeAnnotation',
typeAnnotation: {
id: {
name: 'UnknownType',
},
},
};

const result = mapperFor(mockElement, t);

expect(t.isMemberExpression(result)).toBe(true);
expect(result.object.name).toBe('parsers');
expect(result.property.name).toBe('objectParser');
});
});
});
1 change: 1 addition & 0 deletions src/Options/Definitions.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,7 @@ module.exports.ParseServerOptions = {
env: 'PARSE_SERVER_AUTH_PROVIDERS',
help:
'Configuration for your authentication providers, as stringified JSON. See http://docs.parseplatform.org/parse-server/guide/#oauth-and-3rd-party-authentication',
action: parsers.objectParser,
},
cacheAdapter: {
env: 'PARSE_SERVER_CACHE_ADAPTER',
Expand Down
Loading