Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "prerelease",
"comment": "feat: implements custom JSX pragma",
"packageName": "@fluentui/react-jsx-runtime",
"email": "bernardo.sunderhus@gmail.com",
"dependentChangeType": "patch"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "minor",
"comment": "feat: ensure compatibility with custom JSX pragma",
"packageName": "@fluentui/react-utilities",
"email": "bernardo.sunderhus@gmail.com",
"dependentChangeType": "patch"
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,14 @@

```ts

import { Fragment } from 'react';
import * as React_2 from 'react';

// @public (undocumented)
export function createElement<P extends {}>(type: React_2.ElementType<P>, props?: P | null, ...children: React_2.ReactNode[]): React_2.ReactElement<P> | null;

export { Fragment }

// (No @packageDocumentation comment for this package)

```
2 changes: 1 addition & 1 deletion packages/react-components/react-jsx-runtime/package.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
{
"name": "@fluentui/react-jsx-runtime",
"version": "9.0.0-alpha.0",
"private": true,
"description": "React components for building web experiences",
"main": "lib-commonjs/index.js",
"module": "lib/index.js",
Expand Down Expand Up @@ -30,6 +29,7 @@
"@fluentui/scripts-tasks": "*"
},
"dependencies": {
"@fluentui/react-utilities": "^9.7.4",
"@swc/helpers": "^0.4.14"
},
"peerDependencies": {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,168 @@
/* eslint-disable jsdoc/check-tag-names */
/** @jsxRuntime classic */
/** @jsxFrag Fragment */
/** @jsx createElement */
/* eslint-enable jsdoc/check-tag-names */

import { render } from '@testing-library/react';
import { ComponentProps, ComponentState, Slot, getSlotsNext, resolveShorthand } from '@fluentui/react-utilities';
import { createElement } from './createElement';

describe('createElement', () => {
describe('general behavior tests', () => {
it('handles a string', () => {
const result = render(<div>Hello world</div>);

expect(result.container.firstChild).toMatchInlineSnapshot(`
<div>
Hello world
</div>
`);
});

it('handles an array', () => {
const result = render(
<div>
{Array.from({ length: 3 }, (_, i) => (
<div key={i}>{i}</div>
))}
</div>,
);

expect(result.container.firstChild).toMatchInlineSnapshot(`
<div>
<div>
0
</div>
<div>
1
</div>
<div>
2
</div>
</div>
`);
});

it('handles an array of children', () => {
const result = render(
<div>
<div>1</div>
<div>2</div>
</div>,
);

expect(result.container.firstChild).toMatchInlineSnapshot(`
<div>
<div>
1
</div>
<div>
2
</div>
</div>
`);
});
});

describe('custom behavior tests', () => {
it('keeps children from "defaultProps" in a render callback', () => {
type TestComponentSlots = { slot: Slot<'div'> };
type TestComponentState = ComponentState<TestComponentSlots>;
type TestComponentProps = ComponentProps<Partial<TestComponentSlots>>;

const TestComponent = (props: TestComponentProps) => {
const state: TestComponentState = {
components: { slot: 'div' },

slot: resolveShorthand(props.slot, {
defaultProps: { children: 'Default Children', id: 'slot' },
}),
};
const { slots, slotProps } = getSlotsNext<TestComponentSlots>(state);

return <slots.slot {...slotProps.slot} />;
};

const children = jest.fn().mockImplementation((Component, props) => (
<div id="render-fn">
<Component {...props} />
</div>
));
const result = render(<TestComponent slot={{ children }} />);

expect(children).toHaveBeenCalledTimes(1);
expect(children).toHaveBeenCalledWith('div', { children: 'Default Children', id: 'slot' });

expect(result.container.firstChild).toMatchInlineSnapshot(`
<div
id="render-fn"
>
<div
id="slot"
>
Default Children
</div>
</div>
`);
});

it('keeps children from a render template in a render callback', () => {
type TestComponentSlots = { outer: Slot<'div'>; inner: Slot<'div'> };
type TestComponentState = ComponentState<TestComponentSlots>;
type TestComponentProps = ComponentProps<Partial<TestComponentSlots>>;

const TestComponent = (props: TestComponentProps) => {
const state: TestComponentState = {
components: { inner: 'div', outer: 'div' },

inner: resolveShorthand(props.inner, { defaultProps: { id: 'inner' } }),
outer: resolveShorthand(props.outer, { defaultProps: { id: 'outer' } }),
};
const { slots, slotProps } = getSlotsNext<TestComponentSlots>(state);

return (
<slots.outer {...slotProps.outer}>
<slots.inner {...slotProps.inner} />
</slots.outer>
);
};

const children = jest.fn().mockImplementation((Component, props) => (
<div id="render-fn">
<Component {...props} />
</div>
));
const result = render(<TestComponent outer={{ children }} inner={{ children: 'Inner children' }} />);

expect(children).toHaveBeenCalledTimes(1);
expect(children.mock.calls[0][0]).toBe('div');
expect(children.mock.calls[0][1].id).toBe('outer');
expect(children.mock.calls[0][1].children).toMatchInlineSnapshot(`
<React.Fragment>
<div
id="inner"
>
Inner children
</div>
</React.Fragment>
`);

expect(result.container.firstChild).toMatchInlineSnapshot(`
<div
id="render-fn"
>
<div
id="outer"
>
<div
id="inner"
>
Inner children
</div>
</div>
</div>
`);
});
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
import * as React from 'react';
import { SlotRenderFunction, UnknownSlotProps, SLOT_RENDER_FUNCTION_SYMBOL } from '@fluentui/react-utilities';

type WithMetadata<Props extends {}> = Props & {
[SLOT_RENDER_FUNCTION_SYMBOL]: SlotRenderFunction<Props>;
};

export function createElement<P extends {}>(
type: React.ElementType<P>,
props?: P | null,
...children: React.ReactNode[]
): React.ReactElement<P> | null {
if (!isSlotComponent(props)) {
return React.createElement(type, props, ...children);
}

const result = normalizeRenderFunction(props, children);
return React.createElement(
React.Fragment,
{},
result.renderFunction(type, { ...result.props, children: result.children }),
) as React.ReactElement<P>;
}

function normalizeRenderFunction<Props extends UnknownSlotProps>(
propsWithMetadata: WithMetadata<Props>,
overrideChildren?: React.ReactNode[],
): {
props: Props;
children: React.ReactNode;
renderFunction: SlotRenderFunction<Props>;
} {
const { [SLOT_RENDER_FUNCTION_SYMBOL]: renderFunction, children: externalChildren, ...props } = propsWithMetadata;

const children: React.ReactNode =
Array.isArray(overrideChildren) && overrideChildren.length > 0
? React.createElement(React.Fragment, {}, ...overrideChildren)
: externalChildren;

return {
children,
renderFunction,
props: props as UnknownSlotProps as Props,
};
}
Comment on lines +8 to +45
Copy link
Contributor

Choose a reason for hiding this comment

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

It seems like these functions could be simplified quite a bit, to reduce the number of Fragments and object spreads, and to be easier to understand:

export function createElement<P extends {}>(
  type: React.ElementType<P>,
  props?: P | null,
  ...children: React.ReactNode[]
): React.ReactElement<P> | null {
  if (!isSlotComponent(props)) {
    return React.createElement(type, props, children);
  }

  const { [SLOT_RENDER_FUNCTION_SYMBOL]: renderFunction, ...renderProps } = props;

  if (Array.isArray(children) && children.length > 0) {
    renderProps.children = children;
  }

  return React.createElement(React.Fragment, {}, renderFunction(type, renderProps));
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

renderProps.children cannot be an array! it has to be converted to a fragment. otherwise we'll be requiring for the render function to properly provide keys for each children element.

in this simplification, renderProps.children might end up being an array.

Our tests are even catching this problem:

image

Nonetheless I can see clear simplifications with this method. I'll see to follow up on this with another PR


export function isSlotComponent<Props extends {}>(props?: Props | null): props is WithMetadata<Props> {
return Boolean(props?.hasOwnProperty(SLOT_RENDER_FUNCTION_SYMBOL));
}
4 changes: 2 additions & 2 deletions packages/react-components/react-jsx-runtime/src/index.ts
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
// TODO: replace with real exports
export {};
export { createElement } from './createElement';
export { Fragment } from 'react';
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,12 @@ export function getSlots<R extends SlotPropsRecord>(state: ComponentState<R>): {
slotProps: ObjectSlotProps<R>;
};

// @public
export function getSlotsNext<R extends SlotPropsRecord>(state: ComponentState<R>): {
slots: Slots<R>;
slotProps: ObjectSlotProps<R>;
};

// @internal
export function getTriggerChild<TriggerChildProps>(children: TriggerProps<TriggerChildProps>['children']): (React_2.ReactElement<Partial<TriggerChildProps>> & {
ref?: React_2.Ref<any>;
Expand Down Expand Up @@ -143,6 +149,9 @@ export type Slot<Type extends keyof JSX.IntrinsicElements | React_2.ComponentTyp
} & WithSlotRenderFunction<IntrinsicElementProps<As>>;
}[AlternateAs] | null : 'Error: First parameter to Slot must not be not a union of types. See documentation of Slot type.';

// @internal
export const SLOT_RENDER_FUNCTION_SYMBOL: unique symbol;

// @public
export type SlotClassNames<Slots> = {
[SlotName in keyof Slots]-?: string;
Expand Down Expand Up @@ -175,6 +184,11 @@ export type TriggerProps<TriggerChildProps = unknown> = {
children?: React_2.ReactElement | ((props: TriggerChildProps) => React_2.ReactElement | null) | null;
};

// @public
export type UnknownSlotProps = Pick<React_2.HTMLAttributes<HTMLElement>, 'children' | 'className' | 'style'> & {
as?: keyof JSX.IntrinsicElements;
};

// @internal
export const useControllableState: <State>(options: UseControllableStateOptions<State>) => [State, React_2.Dispatch<React_2.SetStateAction<State>>];

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
/**
* @internal
* Internal reference for the render function
*/
export const SLOT_RENDER_FUNCTION_SYMBOL = Symbol('fui.slotRenderFunction');
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import type {
SlotPropsRecord,
SlotRenderFunction,
UnionToIntersection,
UnknownSlotProps,
} from './types';

export type Slots<S extends SlotPropsRecord> = {
Expand All @@ -19,7 +20,7 @@ export type Slots<S extends SlotPropsRecord> = {
: React.ElementType<ExtractSlotProps<S[K]>>;
};

type ObjectSlotProps<S extends SlotPropsRecord> = {
export type ObjectSlotProps<S extends SlotPropsRecord> = {
[K in keyof S]-?: ExtractSlotProps<S[K]> extends AsIntrinsicElement<infer As>
? // For intrinsic element types, return the intersection of all possible
// element's props, to be compatible with the As type returned by Slots<>
Expand Down Expand Up @@ -68,10 +69,13 @@ function getSlot<R extends SlotPropsRecord, K extends keyof R>(
state: ComponentState<R>,
slotName: K,
): readonly [React.ElementType<R[K]> | null, R[K]] {
if (state[slotName] === undefined) {
const props = state[slotName];

if (props === undefined) {
return [null, undefined as R[K]];
}
const { children, as: asProp, ...rest } = state[slotName]!;

const { children, as: asProp, ...rest } = props;

const slot = (
state.components?.[slotName] === undefined || typeof state.components[slotName] === 'string'
Expand All @@ -89,8 +93,7 @@ function getSlot<R extends SlotPropsRecord, K extends keyof R>(
];
}

const shouldOmitAsProp = typeof slot === 'string' && state[slotName]?.as;
const slotProps = (shouldOmitAsProp ? omit(state[slotName]!, ['as']) : state[slotName]) as R[K];

const shouldOmitAsProp = typeof slot === 'string' && asProp;
const slotProps = (shouldOmitAsProp ? omit(props, ['as']) : (props as UnknownSlotProps)) as R[K];
return [slot, slotProps];
}
Loading