-
-
Notifications
You must be signed in to change notification settings - Fork 4.8k
feat: Update config to automatically check types #8535
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.
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
base: alpha
Are you sure you want to change the base?
Conversation
I will reformat the title to use the proper commit message syntax. |
Thanks for opening this pull request! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Amazing PR and clean-up!
@@ -20,7 +20,7 @@ | |||
* @property {Adapter<AnalyticsAdapter>} analyticsAdapter Adapter module for the analytics | |||
* @property {String} appId Your Parse Application ID | |||
* @property {String} appName Sets the app name | |||
* @property {AuthAdapter[]} auth Configuration for your authentication providers, as stringified JSON. See http://docs.parseplatform.org/parse-server/guide/#oauth-and-3rd-party-authentication | |||
* @property {Auth} auth Configuration for your authentication providers, as stringified JSON. See http://docs.parseplatform.org/parse-server/guide/#oauth-and-3rd-party-authentication |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you give some insight about this change? It looks like a breaking change, or was the type incorrect?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's an incorrect type, the Auth option is not an array, it's an object with keys
:
auth: {
facebook: {},
myAuth: {},
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you please check:
- What do the docs (API docs and in
docs
repo) currently say - array or object? - Are there any tests that currently use an array, or do they all use an object?
- If a developer currently set an array instead of object, would Parse Server accept that option and would that actually work? If it works, will this change break an existing Parse Server where an array is set?
Just so we understand the full implications of this change / bug fix (?).
Signed-off-by: Daniel <[email protected]>
Pull Request
Issue
Parse Server options need to validated manually, where they should be compared directly against their
definitions
.Closes: #8202
Approach
Tasks