-
Notifications
You must be signed in to change notification settings - Fork 127
Internal Server Error fix1 #326
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: main
Are you sure you want to change the base?
Conversation
|
update: Yes, yes it does! Hey Aryan, thanks for your contribution! Does this fix #325? |
|
I had created a test case file that was verifying the fix . I think it should have fix the #325 issue but if any error shows it, do tell me |
|
praise: this approach looks like it'll work thought: I'm worries that no longer using Here's the export function transformObject<T extends Record<string, any>, U>(
object: T,
transform: (value: T[keyof T], key: keyof T) => U | undefined,
): { [K in keyof T]: U } {
const result = {} as { [K in keyof T]: U };
for (const [key, value] of Object.entries(object) as [keyof T, T[keyof T]][]) {
const t = transform(value, key);
if (typeof t !== "undefined") {
result[key] = t;
}
}
return result;
}
export const objectTransform = transformObject; |
|
Well i can update the code to use |
Let's do that, because I think that may give it higher chances of being merged in by using that existing pattern I appreciate you working on this! |
|
Ok changed to the original transformObject. Hope this works! |
|
looks good to me! I'll now defer to Dennis for final review He's on holiday rest at the time of posting this |
|
Yeah sure! Thanks, I'll hop on to another issue in the mean time.! |
|
hey @Aryan-Shrivastva, thanks a lot for taking the time to tackle this. Your approach works, and looks generally good. However, I'm a bit afraid of keeping this maintained, also considering that it currently only solves the issue on the timestamps plugin. Consequently, we would add something like this for relations, e.g. when a plugin adds a new entity. Plugins should be self-contained as much as possible to reduce the surface of error, and prevent headaches in the future. Generally I see 2 ways of solving a situation like this:
The first one is not really ideal, as this means your schema is spread (you add your schema in the |
AppData.build() - Modified to defer index creation when fields don't exist yet.
AppData.resolvePendingIndices() - New method to resolve deferred indices after plugins run.
App.onModulesBuilt() - Updated to call resolvePendingIndices() after plugin schemas are merged.