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

Unsafe argument of type `any` on presigned post fields #1011

Closed reefdog closed 2 months ago

reefdog commented 2 months ago

When using the PresignedPostRequestPresignedPost type, accessing the values from a presigned post fields object triggers the ol' unsafe any type complaint:

Unsafe argument of type `any` assigned to a parameter of type `string | Blob`.

E.g.:

const uploadUsingPresignedPost = async (
    file: File,
    presignedPost: PresignedPostRequestPresignedPost,
) => {
    const formData = new FormData();
    Object.entries(presignedPost.fields).forEach(([key, value]) =>
        // ❌ This is where things break:
        formData.append(key, value),
    );
    // Rest of function omitted
}

Unsure if this should be part of https://github.com/PhilanthropyDataCommons/service/issues/1004 or is separate.

slifty commented 2 months ago

Now that I'm actually digging into this, this is due to the signature of Object.entries, not due to our type definition.

Object.entries cannot safely guarantee that values are always of a given type -- even if the "Type" of a given variable has only string entities documented, TypeScript is not saying there aren't more than those entities on the object (nor does it indicate what those entities will be).

So I think what we'll need to do is add a type guard e.g.:

    const isStringOrBlob(value: unknown) value is string | Blob  => typeof value  === 'string' || value instanceof Blob

    Object.entries(presignedPost.fields).forEach(([key, value]) =>
        if (isValidFormDataValue(value) {
            formData.append(key, value),
        }
    );

Here's the definition of ObjectConstructor.entries in es2017:

    /**
     * Returns an array of key/values of the enumerable properties of an object
     * @param o Object that contains the properties and methods. This can be an object that you created or an existing Document Object Model (DOM) object.
     */
    entries(o: {}): [string, any][];

^ note the any there

slifty commented 2 months ago

For now I'm marking this as closed as not planned based on the above analysis, that said we can absolutely re-open if that is wrong.

reefdog commented 2 months ago

This makes sense! Thanks for the thorough explanation and even tackling the front-end issue that this is linked to! 🙏