PhilanthropyDataCommons / service

A project for collecting and serving public information associated with grant applications
GNU Affero General Public License v3.0
8 stars 2 forks source link

`WritablePresignedPostRequest` improperly requiring readonly property #1012

Closed reefdog closed 2 months ago

reefdog commented 2 months ago

When trying to use an argument of WritablePresignedPostRequest type — which should only require fileType and fileSize — TypeScript gets bigmad that we aren't including presignedPost as well.

This API-access hook should work:

const usePresignedPostCallback = () => {
    const api = usePdcCallbackApi<PresignedPostRequest>('/presignedPostRequests');
    return (params: WritablePresignedPostRequest) =>
        api({
            method: 'post',
            headers: {
                Accept: 'application/json',
                'Content-Type': 'application/json',
            },
            body: JSON.stringify(params),
        });
};

And would be called thusly:

const createPresignedPost = usePresignedPostCallback();
const { presignedPost } = await createPresignedPost({
    fileType: file.type || 'application/octet-stream',
    fileSize: file.size,
});

However, that implementation throws this error:

Argument of type '{ fileType: string; fileSize: number; }' is not assignable to parameter of type '{ fileType: string; fileSize: number; presignedPost: { fields: { key: string; }; url: string; }; }'. Property 'presignedPost' is missing in type '{ fileType: string; fileSize: number; }' but required in type '{ fileType: string; fileSize: number; presignedPost: { fields: { key: string; }; url: string; }; }'.

Our temporary fix on the front end was to instead define and use this local type in the callback:

type FixedWritablePresignedPostRequest = Omit<
    WritablePresignedPostRequest,
    'presignedPost'
>;
slifty commented 2 months ago

Grumble grumble.

So this is properly marked as readonly: https://github.com/PhilanthropyDataCommons/service/blob/5795b15786e3c3d098af6522b8d61d5e6de3b386/src/openapi.json#L250

However it looks like swagger codegen is not honoring that fact. I'm going to take a moment trying to figure out the state of reality with swagger codegen right now to make sure we're on the latest version.

slifty commented 2 months ago

Some related resources:

Some of this might also inform fixes to #1004