Skip to content

Commit

Permalink
fix(JSX): remove Promise from FC
Browse files Browse the repository at this point in the history 
The fact that components are QRLs and return Promises is an implementation detail
Also remove constraints from FunctionComponent props, to make inferral
work better. It's ok that props are now allowed to be non-objects,
because in actuality it just doesn't work and the mistake would be
apparent immediately.
  • Loading branch information
@wmertens
wmertens committed Jan 18, 2024
1 parent 6eec3bd commit 12c13eb
Show file tree
Hide file tree
Showing 7 changed files with 68 additions and 42 deletions.
8 changes: 4 additions & 4 deletions packages/docs/src/routes/api/qwik/api.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -847,7 +847,7 @@
}
],
"kind": "TypeAlias",
"content": "Any sync or async function that returns JSXOutput.\n\nNote that this includes QRLs.\n\nThe `key`<!-- -->, `flags` and `dev` parameters are for internal use.\n\n\n```typescript\nexport type FunctionComponent<P extends Record<any, any> = Record<any, unknown>> = {\n renderFn(props: P, key: string | null, flags: number, dev?: DevJSX): JSXOutput | Promise<JSXOutput>;\n}['renderFn'];\n```\n**References:** [DevJSX](#devjsx)<!-- -->, [JSXOutput](#jsxoutput)",
"content": "Any function taking a props object that returns JSXOutput.\n\nThe `key`<!-- -->, `flags` and `dev` parameters are for internal use.\n\n\n```typescript\nexport type FunctionComponent<P = unknown> = {\n renderFn(props: P, key: string | null, flags: number, dev?: DevJSX): JSXOutput;\n}['renderFn'];\n```\n**References:** [DevJSX](#devjsx)<!-- -->, [JSXOutput](#jsxoutput)",
"editUrl": "https://github.com/BuilderIO/qwik/tree/main/packages/qwik/src/core/render/jsx/types/jsx-node.ts",
"mdFile": "qwik.functioncomponent.md"
},
Expand Down Expand Up @@ -1158,7 +1158,7 @@
}
],
"kind": "Variable",
"content": "```typescript\njsx: <T extends string | FunctionComponent<any>>(type: T, props: T extends FunctionComponent<infer PROPS extends Record<any, any>> ? PROPS : Record<any, unknown>, key?: string | number | null) => JSXNode<T>\n```",
"content": "```typescript\njsx: <T extends string | FunctionComponent<any>>(type: T, props: T extends FunctionComponent<infer PROPS extends Record<any, any> | undefined> ? PROPS : Record<any, unknown>, key?: string | number | null) => JSXNode<T>\n```",
"editUrl": "https://github.com/BuilderIO/qwik/tree/main/packages/qwik/src/core/render/jsx/jsx-runtime.ts",
"mdFile": "qwik.jsx.md"
},
Expand Down Expand Up @@ -1186,7 +1186,7 @@
}
],
"kind": "Variable",
"content": "```typescript\njsxDEV: <T extends string | FunctionComponent>(type: T, props: T extends FunctionComponent<infer PROPS extends Record<any, any>> ? PROPS : Record<any, unknown>, key: string | number | null | undefined, _isStatic: boolean, opts: JsxDevOpts, _ctx: unknown) => JSXNode<T>\n```",
"content": "```typescript\njsxDEV: <T extends string | FunctionComponent>(type: T, props: T extends FunctionComponent<infer PROPS extends Record<any, any> | undefined> ? PROPS : Record<any, unknown>, key: string | number | null | undefined, _isStatic: boolean, opts: JsxDevOpts, _ctx: unknown) => JSXNode<T>\n```",
"editUrl": "https://github.com/BuilderIO/qwik/tree/main/packages/qwik/src/core/render/jsx/jsx-runtime.ts",
"mdFile": "qwik.jsxdev.md"
},
Expand All @@ -1200,7 +1200,7 @@
}
],
"kind": "Interface",
"content": "A JSX Node, an internal structure. You probably want to use `JSXOutput` instead.\n\n\n```typescript\nexport interface JSXNode<T extends string | FunctionComponent | unknown = unknown> \n```\n\n\n| Property | Modifiers | Type | Description |\n| --- | --- | --- | --- |\n| [children](#) | | [JSXChildren](#jsxchildren) \\| null | |\n| [dev?](#) | | [DevJSX](#devjsx) | _(Optional)_ |\n| [flags](#) | | number | |\n| [immutableProps](#) | | Record&lt;any, unknown&gt; \\| null | |\n| [key](#) | | string \\| null | |\n| [props](#) | | T extends [FunctionComponent](#functioncomponent)<!-- -->&lt;infer B&gt; ? B : Record&lt;any, unknown&gt; | |\n| [type](#) | | T | |",
"content": "A JSX Node, an internal structure. You probably want to use `JSXOutput` instead.\n\n\n```typescript\nexport interface JSXNode<T extends string | FunctionComponent | unknown = unknown> \n```\n\n\n| Property | Modifiers | Type | Description |\n| --- | --- | --- | --- |\n| [children](#) | | [JSXChildren](#jsxchildren) \\| null | |\n| [dev?](#) | | [DevJSX](#devjsx) | _(Optional)_ |\n| [flags](#) | | number | |\n| [immutableProps](#) | | Record&lt;any, unknown&gt; \\| null | |\n| [key](#) | | string \\| null | |\n| [props](#) | | T extends [FunctionComponent](#functioncomponent)<!-- -->&lt;infer P&gt; ? P : Record&lt;any, unknown&gt; | |\n| [type](#) | | T | |",
"editUrl": "https://github.com/BuilderIO/qwik/tree/main/packages/qwik/src/core/render/jsx/types/jsx-node.ts",
"mdFile": "qwik.jsxnode.md"
},
Expand Down
20 changes: 10 additions & 10 deletions packages/docs/src/routes/api/qwik/index.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -866,22 +866,18 @@ Fragment: FunctionComponent<{
## FunctionComponent
Any sync or async function that returns JSXOutput.
Note that this includes QRLs.
Any function taking a props object that returns JSXOutput.
The `key`, `flags` and `dev` parameters are for internal use.
```typescript
export type FunctionComponent<
P extends Record<any, any> = Record<any, unknown>,
> = {
export type FunctionComponent<P = unknown> = {
renderFn(
props: P,
key: string | null,
flags: number,
dev?: DevJSX,
): JSXOutput | Promise<JSXOutput>;
): JSXOutput;
}["renderFn"];
```
Expand Down Expand Up @@ -1223,7 +1219,9 @@ isSignal: <T = unknown>(obj: any) => obj is Signal<T>
```typescript
jsx: <T extends string | FunctionComponent<any>>(
type: T,
props: T extends FunctionComponent<infer PROPS extends Record<any, any>>
props: T extends FunctionComponent<
infer PROPS extends Record<any, any> | undefined
>
? PROPS
: Record<any, unknown>,
key?: string | number | null,
Expand Down Expand Up @@ -1258,7 +1256,9 @@ export type JSXChildren =
```typescript
jsxDEV: <T extends string | FunctionComponent>(
type: T,
props: T extends FunctionComponent<infer PROPS extends Record<any, any>>
props: T extends FunctionComponent<
infer PROPS extends Record<any, any> | undefined
>
? PROPS
: Record<any, unknown>,
key: string | number | null | undefined,
Expand All @@ -1285,7 +1285,7 @@ export interface JSXNode<T extends string | FunctionComponent | unknown = unknow
| [flags](#) | | number | |
| [immutableProps](#) | | Record&lt;any, unknown&gt; \| null | |
| [key](#) | | string \| null | |
| [props](#) | | T extends [FunctionComponent](#functioncomponent)&lt;infer B&gt; ? B : Record&lt;any, unknown&gt; | |
| [props](#) | | T extends [FunctionComponent](#functioncomponent)&lt;infer P&gt; ? P : Record&lt;any, unknown&gt; | |
| [type](#) | | T | |
[Edit this section](https://github.com/BuilderIO/qwik/tree/main/packages/qwik/src/core/render/jsx/types/jsx-node.ts)
Expand Down
12 changes: 6 additions & 6 deletions packages/qwik/src/core/api.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -247,8 +247,8 @@ export const Fragment: FunctionComponent<{
}>;

// @public
export type FunctionComponent<P extends Record<any, any> = Record<any, unknown>> = {
renderFn(props: P, key: string | null, flags: number, dev?: DevJSX): JSXOutput | Promise<JSXOutput>;
export type FunctionComponent<P = unknown> = {
renderFn(props: P, key: string | null, flags: number, dev?: DevJSX): JSXOutput;
}['renderFn'];

// @internal (undocumented)
Expand Down Expand Up @@ -394,7 +394,7 @@ export type IntrinsicSVGElements = {
export const isSignal: <T = unknown>(obj: any) => obj is Signal<T>;

// @public (undocumented)
const jsx: <T extends string | FunctionComponent<any>>(type: T, props: T extends FunctionComponent<infer PROPS extends Record<any, any>> ? PROPS : Record<any, unknown>, key?: string | number | null) => JSXNode<T>;
const jsx: <T extends string | FunctionComponent<any>>(type: T, props: T extends FunctionComponent<infer PROPS extends Record<any, any> | undefined> ? PROPS : Record<any, unknown>, key?: string | number | null) => JSXNode<T>;
export { jsx }
export { jsx as jsxs }

Expand All @@ -404,13 +404,13 @@ export const _jsxBranch: <T>(input?: T | undefined) => T | undefined;
// Warning: (ae-forgotten-export) The symbol "JsxDevOpts" needs to be exported by the entry point index.d.ts
//
// @internal
export const _jsxC: <T extends string | FunctionComponent>(type: T, mutableProps: (T extends FunctionComponent<infer PROPS extends Record<any, any>> ? PROPS : Record<any, unknown>) | null, flags: number, key: string | number | null, dev?: JsxDevOpts) => JSXNode<T>;
export const _jsxC: <T extends string | FunctionComponent>(type: T, mutableProps: (T extends FunctionComponent<infer PROPS extends Record<any, any> | undefined> ? PROPS : Record<any, unknown>) | null, flags: number, key: string | number | null, dev?: JsxDevOpts) => JSXNode<T>;

// @public (undocumented)
export type JSXChildren = string | number | boolean | null | undefined | Function | RegExp | JSXChildren[] | Promise<JSXChildren> | Signal<JSXChildren> | JSXNode;

// @public (undocumented)
export const jsxDEV: <T extends string | FunctionComponent>(type: T, props: T extends FunctionComponent<infer PROPS extends Record<any, any>> ? PROPS : Record<any, unknown>, key: string | number | null | undefined, _isStatic: boolean, opts: JsxDevOpts, _ctx: unknown) => JSXNode<T>;
export const jsxDEV: <T extends string | FunctionComponent>(type: T, props: T extends FunctionComponent<infer PROPS extends Record<any, any> | undefined> ? PROPS : Record<any, unknown>, key: string | number | null | undefined, _isStatic: boolean, opts: JsxDevOpts, _ctx: unknown) => JSXNode<T>;

// @public
export interface JSXNode<T extends string | FunctionComponent | unknown = unknown> {
Expand All @@ -425,7 +425,7 @@ export interface JSXNode<T extends string | FunctionComponent | unknown = unknow
// (undocumented)
key: string | null;
// (undocumented)
props: T extends FunctionComponent<infer B> ? B : Record<any, unknown>;
props: T extends FunctionComponent<infer P> ? P : Record<any, unknown>;
// (undocumented)
type: T;
}
Expand Down
1 change: 1 addition & 0 deletions packages/qwik/src/core/component/component.public.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ export type PropsOf<COMP> = COMP extends string
*
* @public
*/
// In reality, Component is a QRL but that makes the types too complex
export type Component<PROPS = unknown> = FunctionComponent<PublicProps<PROPS>>;

export type ComponentChildren<PROPS> = PROPS extends {
Expand Down
4 changes: 2 additions & 2 deletions packages/qwik/src/core/render/jsx/jsx-runtime.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ export const _jsxS = <T extends string>(
*
* Create a JSXNode for any tag, with possibly immutable props embedded in props
*/
export const _jsxC = <T extends string | FunctionComponent>(
export const _jsxC = <T extends string | FunctionComponent<Record<any, unknown>>>(
type: T,
mutableProps: (T extends FunctionComponent<infer PROPS> ? PROPS : Record<any, unknown>) | null,
flags: number,
Expand Down Expand Up @@ -367,7 +367,7 @@ export const HTMLFragment: FunctionComponent<{ dangerouslySetInnerHTML: string }
jsx(Virtual, props);

/** @public */
export const jsxDEV = <T extends string | FunctionComponent>(
export const jsxDEV = <T extends string | FunctionComponent<Record<any, unknown>>>(
type: T,
props: T extends FunctionComponent<infer PROPS> ? PROPS : Record<any, unknown>,
key: string | number | null | undefined,
Expand Down
15 changes: 4 additions & 11 deletions packages/qwik/src/core/render/jsx/types/jsx-node.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,21 +8,14 @@ import type { JSXChildren } from './jsx-qwik-attributes';
export type JSXOutput = JSXNode | string | number | boolean | null | undefined | JSXOutput[];

/**
* Any sync or async function that returns JSXOutput.
*
* Note that this includes QRLs.
* Any function taking a props object that returns JSXOutput.
*
* The `key`, `flags` and `dev` parameters are for internal use.
*
* @public
*/
export type FunctionComponent<P extends Record<any, any> = Record<any, unknown>> = {
renderFn(
props: P,
key: string | null,
flags: number,
dev?: DevJSX
): JSXOutput | Promise<JSXOutput>;
export type FunctionComponent<P = unknown> = {
renderFn(props: P, key: string | null, flags: number, dev?: DevJSX): JSXOutput;
}['renderFn'];

/** @public */
Expand All @@ -40,7 +33,7 @@ export interface DevJSX {
*/
export interface JSXNode<T extends string | FunctionComponent | unknown = unknown> {
type: T;
props: T extends FunctionComponent<infer B> ? B : Record<any, unknown>;
props: T extends FunctionComponent<infer P> ? P : Record<any, unknown>;
immutableProps: Record<any, unknown> | null;
children: JSXChildren | null;
flags: number;
Expand Down
50 changes: 41 additions & 9 deletions packages/qwik/src/core/render/jsx/types/jsx-types.unit.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import type { QwikIntrinsicElements } from './jsx-qwik-elements';
import type { JSXChildren } from './jsx-qwik-attributes';
import { component$, type PropsOf, type PublicProps } from '../../../component/component.public';
import type { QwikHTMLElements, QwikSVGElements, Size } from './jsx-generated';
import type { QwikJSX } from './jsx-qwik';
import type { JSX } from '../jsx-runtime';

describe('types', () => {
// Note, these type checks happen at compile time. We don't need to call anything, so we do ()=>()=>. We just need to
Expand Down Expand Up @@ -49,6 +49,36 @@ describe('types', () => {
expectTypeOf(Cmp).not.toEqualTypeOf<FunctionComponent<{ foo: string }>>();
});

test('inferring FunctionComponent', () => () => {
const makeFC = <T,>(fn: (p: T) => JSX.Element): FunctionComponent<T> => fn;

const FCNoP = makeFC(() => <div />);
expectTypeOf<PropsOf<typeof FCNoP>>().toEqualTypeOf<never>();

const FCWithP = makeFC((p) => {
expectTypeOf(p).not.toBeAny();
expectTypeOf(p).toEqualTypeOf<unknown>();
return <div />;
});

expectTypeOf<PropsOf<typeof FCWithP>>().toEqualTypeOf<never>();
});

test('accepting FunctionComponent', () => () => {
const f = <P,>(fn: FunctionComponent<P>) => null as P;
expectTypeOf(f(() => <div />)).toEqualTypeOf<unknown>();
expectTypeOf(
f((p) => {
expectTypeOf(p).not.toBeAny();
expectTypeOf(p).toEqualTypeOf<unknown>();
return <div />;
})
).toEqualTypeOf<unknown>();
expectTypeOf(f(component$<{ foo: string }>(() => <div />))).toEqualTypeOf<
PublicProps<{ foo: string }>
>();
});

test('component', () => () => {
const Cmp = component$((props: PropsOf<'svg'>) => {
const { width = '240', height = '56', onClick$, ...rest } = props;
Expand All @@ -66,6 +96,12 @@ describe('types', () => {
expectTypeOf<Parameters<typeof Cmp>[0]['onClick$']>().toMatchTypeOf<
EventHandler<PointerEvent, SVGSVGElement> | QRLEventHandlerMulti<PointerEvent, SVGSVGElement>
>();

return (
<p>
<Cmp />
</p>
);
});
test('PropFunction', () => () => {
const CmpButton = component$<{
Expand All @@ -87,7 +123,7 @@ describe('types', () => {
}}
/>
);
expectTypeOf(t).toEqualTypeOf<QwikJSX.Element>();
expectTypeOf(t).toEqualTypeOf<JSX.Element>();
});

test('inferring', () => () => {
Expand Down Expand Up @@ -175,6 +211,8 @@ describe('types', () => {
Parameters<Extract<Parameters<typeof Poly>[0]['onClick$'], EventHandler>>[1]
>().toEqualTypeOf<HTMLDivElement>();

const MyCmp = component$((p: { name: string }) => <span>Hi {p.name}</span>);

return (
<>
<Poly
Expand Down Expand Up @@ -213,12 +251,7 @@ describe('types', () => {
Bar
</Poly>
<Poly as={Poly} />
<Poly
as={$((p: { name: string }) => (
<span>Hi {p.name}</span>
))}
name="meep"
/>
<Poly as={MyCmp} name="meep" />
</>
);
});
Expand Down Expand Up @@ -248,7 +281,6 @@ describe('types', () => {
expectTypeOf(() => null).toMatchTypeOf<FunctionComponent<{ hi: number }>>();

expectTypeOf(() => new Date()).not.toMatchTypeOf<FunctionComponent>();
expectTypeOf((p: string) => null).not.toMatchTypeOf<FunctionComponent>();
});

test('PropsOf', () => () => {
Expand Down

0 comments on commit 12c13eb

Please sign in to comment.