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

[✨] Add way to specify PLATFORM for action$ #2952

Closed
mewhhaha opened this issue Feb 12, 2023 · 2 comments
Closed

[✨] Add way to specify PLATFORM for action$ #2952

mewhhaha opened this issue Feb 12, 2023 · 2 comments
Labels
STATUS-1: needs triage New issue which needs to be triaged TYPE: enhancement New feature or request
Milestone

Comments

@mewhhaha
Copy link

mewhhaha commented Feb 12, 2023

Is your feature request related to a problem?

Love using qwik! I hope I'm doing this correctly.

export interface ActionConstructor {
  <O>(actionQrl: (form: JSONObject, event: RequestEventAction) => ValueOrPromise<O>): Action<O>;
  <O, B extends ZodReturn>(
    actionQrl: (data: GetValidatorType<B>, event: RequestEventAction) => ValueOrPromise<O>,
    options: B
  ): Action<
    O | FailReturn<z.typeToFlattenedError<GetValidatorType<B>>>,
    GetValidatorType<B>,
    false
  >;
}

In the type signature for action$ the PLATFORM for RequestEventAction defaults to unknown, unlike in loader$ where you can pass a generic type.

Describe the solution you'd like

Add a generic type to action$ that gets passed to RequestEventAction so that you can specify the PLATFORM. The same way that is possible to do it for loader$.

OR

Similar to Remix have both loader$ and action$ use an empty interface that is exposed for PLATFORM instead of a generic type. This would make it so that you can make one declaration app-wide. (link to Remix MR that did something similar)[https://github.com/remix-run/remix/pull/1876 ].

In Remix they have AppLoadContext that is their equivalent of PLATFORM. By including a declaration file like below you can merge it and have AppLoadContext be whatever you want in the application. So you don't have to type AppLoadContext at each instance when you use a loader or an action.

declare module "@remix-run/cloudflare" {
  interface AppLoadContext {
    ...
  }
}

Describe alternatives you've considered

  • Casting platform to the proper type.
  • @ts-ignore the error
  • Using a patching tool like pnpm patch

Additional context

No response

@mewhhaha mewhhaha added TYPE: enhancement New feature or request STATUS-1: needs triage New issue which needs to be triaged labels Feb 12, 2023
@mewhhaha mewhhaha changed the title [✨] Add generic type to specify PLATFORM for action$ [✨] Add way to specify PLATFORM for action$ Feb 13, 2023
@manucorporat manucorporat added this to the 0.15.0 milestone Feb 13, 2023
@mewhhaha
Copy link
Author

I tried out qwik-city 0.2.1 which seems to have resolved this issue with the updated definition. 🎉 Thank you!

export declare interface RequestEventAction<PLATFORM = QwikCityPlatform> extends RequestEventCommon<PLATFORM> {
    fail: <T extends Record<string, any>>(status: number, returnData: T) => FailReturn<T>;
}

Given the definition above I added the below to a declaration file and I was able to get typings for platform in action$

import "@builder.io/qwik-city/middleware/request-handler";

declare module "@builder.io/qwik-city/middleware/request-handler" {
  interface QwikCityPlatform {
    foobar: string;
  }
}

Screenshot 2023-02-16 at 00 58 14

However it seems that loader$ is not having this properly applied to it. loader$ uses RequestEventLoader_2 which defaults to QwikCityPlatform ❤️ however 😨 the generic for RequestEventLoader_2 is being set by the loader$ function which defaults it to unknown, so even if QwikCityPlatform is defined it ends up being unknown.

export declare const loader$: <RETURN, PLATFORM = unknown>(first: (event: RequestEventLoader_2<PLATFORM>) => RETURN) => Loader<RETURN>;

Screenshot 2023-02-16 at 00 58 23

@zanettin zanettin modified the milestones: 0.15.0, priority Feb 20, 2023
@manucorporat
Copy link
Contributor

This should be fixed already! also for loader, new way does not require developer to provide the type in every loader and action!

Also new server integrations will automaticlaly inject the global type augmentation!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
STATUS-1: needs triage New issue which needs to be triaged TYPE: enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants