Skip to content

Typing missing to allow functionParams defineString to be used for region #5524

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

Closed
RYFN opened this issue Feb 15, 2023 · 2 comments · Fixed by firebase/firebase-functions#1353

Comments

@RYFN
Copy link

RYFN commented Feb 15, 2023

[REQUIRED] Environment info

firebase-tools: 11.22.0

Platform: macOS

[REQUIRED] Test case

const region = defineString('REGION');

export const onCreate = functions
    //results in TS error, see attached image
    .region(region)
    .firestore.document('document/{docId}')
    .onCreate(async (snapshot, context)

218504158-68048fb5-d485-453b-ab89-63691d8b2636

[REQUIRED] Steps to reproduce

Per test case

[REQUIRED] Expected behavior

Should be able to use output of defineString to configure function region

[REQUIRED] Actual behavior

Need to cast result of defineString with as any to allow typescript build. Function works as expected when deployed with

const region = defineString('REGION');

export const onCreate = functions
    .region(region as any)
    .firestore.document('document/{docId}')
    .onCreate(async (snapshot, context)

Follow up to #5347, resolved by https://github.com/firebase/firebase-functions/pull/1329/files suspect adding Expression<string> | to region config would fix this?

(further to this, what's best practice when using defineString in globally defined things in functions? e.g. using the following results in a deployment warning, but not using .value() causes an error)

const redisUsername  = defineString('REDIS_USERNAME') 

//username expects a string
const redisOptions: RedisClientOptions = {
    username: redisUsername.value()
};
... 
export const onCreate = functions
    .region(region as any)
    .firestore.document('document/{docId}')
    .onCreate(async (snapshot, context)
    ...
@colerogers
Copy link
Contributor

colerogers commented Feb 24, 2023

Thanks for pointing this out! I drafted a fix in the SDK and once it's merged we'll release it asap!

@simonmitchell
Copy link

@colerogers any advice on the question regarding globally defined things?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants