Skip to content
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

Parameterized configuration of cors from dotenv needs full TypeScript support #1506

Open
CodingDoug opened this issue Dec 23, 2023 · 4 comments

Comments

@CodingDoug
Copy link

CodingDoug commented Dec 23, 2023

Version info

node: 18

firebase-functions: 4.5.0

firebase-tools: 13.0.2

[REQUIRED] Steps to reproduce

firebase-functions lacks the correct TypeScript types to do what is stated in the documentation for parameterized configuration. If you try the example code:

// Define some parameters
const corsOrigin = defineString('CORS_ORIGIN');

export const fn = onRequest(
  { cors: corsOrigin },
  (req, res) => { ... }
);

TypeScript will fail at compilation with an error like this:

Type 'StringParam' is not assignable to type 'string | boolean | RegExp | (string | RegExp)[] | undefined'.

cors is missing a unioned type for Expression<string> like the other string configurations:

    cors?: string | boolean | RegExp | Array<string | RegExp>;

So it seems impossible to configure cors this way (at least from a compilation perspective - perhaps at runtime the JavaScript will still try to eval a provided StringParam using a hack like corsOrigin as unknown as string?).

Secondly, cors takes RegExp and string arrays, and it would be great to be able to configure it using those variants as well. Cors config can vary greatly based on deployment environment.

@google-oss-bot
Copy link
Collaborator

I found a few problems with this issue:

  • I couldn't figure out how to label this issue, so I've labeled it for a human to triage. Hang tight.
  • This issue does not seem to follow the issue template. Make sure you provide all the required information.

@exaby73
Copy link

exaby73 commented Jan 5, 2024

Hello @CodingDoug. You can use corsOrigin.value() like in the following example:

const corsOrigin = defineString('*')

export const hello = onRequest({
    cors: corsOrigin.value(),
}, request => {
    request.res?.json({hello: 'world'});
});

Does this solve your issue?

@exaby73 exaby73 added Needs: Author Feedback Issues awaiting author feedback and removed needs-triage labels Jan 5, 2024
@CodingDoug
Copy link
Author

CodingDoug commented Jan 5, 2024

@exaby73 No, it doesn't work because corsOrigin.value() gets executed at load time time (before the value is known), not at runtime when the function is invoked. The value() function only works at runtime. That's why the other function function configuration parameters accept a StringParam type object, so that it can hold on and wait to evaluate it later when the value is known.

Take, for example, the TypeScript definition of the HttpsOptions object passed to onCreate, and see how it takes various Expression<> type values (which is what allows StringParams):

export interface HttpsOptions extends Omit<GlobalOptions, "region"> {
    omit?: boolean | Expression<boolean>;
    region?: SupportedRegion | string | Array<SupportedRegion | string> | Expression<string> | ResetValue;
    cors?: string | boolean | RegExp | Array<string | RegExp>;
    memory?: options.MemoryOption | Expression<number> | ResetValue;
    ...
}

See that cors does not accept an Expression<string> unlike the other configs. That's what I'm asking to add.

@google-oss-bot google-oss-bot added Needs: Attention and removed Needs: Author Feedback Issues awaiting author feedback labels Jan 5, 2024
@exaby73
Copy link

exaby73 commented Jan 5, 2024

I understand your use case here. I will label this as a feature request and let additional insights come in from the team. Thank you :)

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

No branches or pull requests

3 participants