Skip to content

fix(storage): add missing type for StringFormat object values#8530

Merged
mikehardy merged 1 commit into
invertase:mainfrom
superguineapig:main
May 12, 2025
Merged

fix(storage): add missing type for StringFormat object values#8530
mikehardy merged 1 commit into
invertase:mainfrom
superguineapig:main

Conversation

@superguineapig

@superguineapig superguineapig commented May 12, 2025

Copy link
Copy Markdown
Contributor

Description

This is a TypeScript Type Only change. The type declarations for the storage module were missing the union type that aggregates the string literal values of the StringFormat "enum" object. Without this type, the uploadString function's 3rd parameter expects an Object (corresponding to the StringFormat interface) instead of a string that specifies the format of the uploaded data. This causes a TypeScript compile/check error, but does not otherwise affect the functionality of the function implementation.

This PR adds the appropriate type declaration to satisfy type checking as described in Firebase docs

Related issues

Release Summary

Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
    • Yes
  • My change supports the following platforms;
    • Android
    • iOS
    • Other (macOS, web)
  • My change includes tests;
    • e2e tests added or updated in packages/\*\*/e2e
    • jest tests added or updated in packages/\*\*/__tests__
  • I have updated TypeScript types that are affected by my change.
  • This is a breaking change;
    • Yes
    • No

Test Plan

I don't believe there is a trivial way to write a Jest test for this issue as it involves a type annotation. In my personal code, a @ts-ignore to suppress the error, with subsequent passing of a valid string for the argument resulted in perfectly functional run-time behavior.

The changes in this PR remove the need for the error suppression comment and the dev-time string inference is performed as expected.

It may be noted that there already exists a run-time value assertion to ensure the value for the format argument is one of the expected strings.

See:

if (!Object.values(StringFormat).includes(format)) {
throw new Error(
`firebase.storage.StorageReference.putString(_, *, _) 'format' provided is invalid, must be one of ${Object.values(
StringFormat,
).join(',')}.`,
);
}

🔥

Think react-native-firebase is great? Please consider supporting the project with any of the below:

@vercel

vercel Bot commented May 12, 2025

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
react-native-firebase ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 12, 2025 10:26pm

@CLAassistant

CLAassistant commented May 12, 2025

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@mikehardy mikehardy left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@superguineapig this looks great, thank you! If you sign the CLA I can release this immediately

edit: signed! Thanks

@mikehardy mikehardy added plugin: storage Firebase Cloud Storage Blocked: Missing CLA Workflow: Pending Merge Waiting on CI or similar and removed Blocked: Missing CLA labels May 12, 2025
@mikehardy mikehardy merged commit 4e75f57 into invertase:main May 12, 2025
11 of 15 checks passed
@mikehardy mikehardy removed the Workflow: Pending Merge Waiting on CI or similar label May 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

plugin: storage Firebase Cloud Storage

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[🔥 ] (Modular) Storage API missing type declaration for StringFormat enum

3 participants