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

refactor: types for withTheme #2279

Closed
wants to merge 18 commits into from
Closed
Show file tree
Hide file tree
Changes from 16 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
22 changes: 22 additions & 0 deletions docs/3.x upgrade guide.md
Expand Up @@ -15,3 +15,25 @@ For a slightly more elaborate setup, [@babel/preset-env](https://babeljs.io/docs
From `@babel/preset-env`'s docs

> We leverage [`browserslist`, `compat-table`, and `electron-to-chromium`] to maintain mappings of which version of our supported target environments gained support of a JavaScript syntax or browser feature, as well as a mapping of those syntaxes and features to Babel transform plugins and core-js polyfills.

### Typescript usage of withTheme

The typings for `withTheme` have been updated to more accurately reflect its nature as a factory function that returns a `Form` component and to properly type the ref forwarded to this component. See more about [the changes here](https://github.com/rjsf-team/react-jsonschema-form/pull/2279)

If you're currently using `withTheme`, the typing for the formData was always `any`, as the generated Form component was not able to accept a generic argument.

In v3.x these typings will properly infer formData if no type is given, or validate it if one is, similar to how the exported Form from core would work.
Copy link
Member

Choose a reason for hiding this comment

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

How is a type "given"? Should we show an example code of how to do so?

Copy link
Contributor

@jorisw jorisw Aug 31, 2021

Choose a reason for hiding this comment

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

Ping @mattcosta7

Would be nice if we can merge this PR.


You can maintain the old behavior of `withTheme`, if necessary, by changing usage of your ThemedForm to override the generic for formData with `any`.

```tsx
const schema = { ... }
const ThemedForm = withTheme({ ... })
const formData = { ... }

// change this
<ThemedForm schema={schema} formData={formData} />

// to this to override the formData type with any and maintain the v2 behavior
<ThemedForm<any> schema={schema} formData={formData} />
```
9 changes: 3 additions & 6 deletions packages/bootstrap-4/src/Form/Form.tsx
@@ -1,10 +1,7 @@
import { withTheme, FormProps } from "@rjsf/core";

import CoreForm from "@rjsf/core";
import { withTheme } from "@rjsf/core";
import Theme from "../Theme";
import { StatelessComponent } from "react";

const Form:
| React.ComponentClass<FormProps<any>>
| StatelessComponent<FormProps<any>> = withTheme(Theme);
const Form: typeof CoreForm = withTheme(Theme);

export default Form;
38 changes: 35 additions & 3 deletions packages/bootstrap-4/test/Form.test.tsx
@@ -1,8 +1,8 @@
import React from "react";
import Form from "../src/index";
import CoreForm, { UiSchema } from "@rjsf/core";
import { JSONSchema7 } from "json-schema";
import React from "react";
import renderer from "react-test-renderer";
import { UiSchema } from "@rjsf/core";
import Form from "../src/index";

describe("single fields", () => {
describe("string field", () => {
Expand Down Expand Up @@ -198,3 +198,35 @@ describe("single fields", () => {
expect(tree).toMatchSnapshot();
});
});

describe("refs", () => {
epicfaace marked this conversation as resolved.
Show resolved Hide resolved
test("Renders with a ref and the Form Element accepts a generic formData argument", () => {
let ref = React.createRef<CoreForm<{
field: string;
}>>()

renderer.create(
<Form<{
field: string;
}>
ref={ref}
formData={{
field: "",
}}
schema={{
type: "object",
properties: {
field: {
type: "string",
},
},
}}
/>
);

expect(ref.current).toBeDefined();
expect(ref.current!.submit).toBeDefined();
expect(ref.current!.formElement).toBeDefined();
expect(ref.current!.validate).toBeDefined();
});
});
6 changes: 3 additions & 3 deletions packages/core/index.d.ts
Expand Up @@ -61,6 +61,7 @@ declare module '@rjsf/core' {
onChange: (formData: T, newErrorSchema: ErrorSchema) => void;
onBlur: (id: string, value: boolean | number | string | null) => void;
submit: () => void;
formElement: HTMLFormElement | null;
epicfaace marked this conversation as resolved.
Show resolved Hide resolved
}

export type UiSchema = {
Expand Down Expand Up @@ -274,7 +275,7 @@ declare module '@rjsf/core' {

export function withTheme<T = any>(
themeProps: ThemeProps<T>,
): React.ComponentClass<FormProps<T>> | React.StatelessComponent<FormProps<T>>;
): typeof Form;
Copy link

Choose a reason for hiding this comment

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

edit: add back children property

Here's my take on this, in use in a company project for a year.
It solves the generics problem too.

For reference, it is then used in a module that has form factory filling specific needs in our project (additional default behavior, external styling additions, data type references in event handlers, error message translation, etc..).

withTheme.tsx

import React, { ReactElement, ReactNode, Ref, RefAttributes, forwardRef } from 'react';

import JsonSchemaForm, { FormProps, ThemeProps } from '@rjsf/core';

type TFormWithTheme = <T = {}>(
  p: FormProps<T> & { readonly children?: ReactNode } & RefAttributes<JsonSchemaForm<T>>
) => ReactElement | null;

export function withTheme<T = {}>(themeProps: ThemeProps<T>): TFormWithTheme {
  return forwardRef(
    (props: FormProps<T> & { readonly children?: ReactNode }, ref: Ref<JsonSchemaForm<T>>) => {
      const { fields, widgets, children, ...directProps } = props;

      return (
        <JsonSchemaForm<T>
          {...themeProps}
          {...directProps}
          fields={{ ...themeProps.fields, ...fields }}
          widgets={{ ...themeProps.widgets, ...widgets }}
          ref={ref}
        >
          {children}
        </JsonSchemaForm>
      );
    }
  ) as TFormWithTheme;
}

usage.tsx

const FormWithMUITheme = withTheme(MuiTheme);

type TForm = <T = {}>(
  p: FormProps<T> & RefAttributes<JsonSchemaForm<T>>
) => ReactElement | null;

export const Form: TForm = forwardRef(
    <T extends unknown = {}>(props: FormProps<T>, ref: Ref<JsonSchemaForm<T>>) => {
      const { whatever, ...otherProps } = props;
    
      /* various operations */

      return (
        <FormWithMUITheme<T>
          ref={ref}
          {...otherProps}
        />
      )
    };
) as TForm;


export type AddButtonProps = {
className: string;
Expand Down Expand Up @@ -436,8 +437,7 @@ declare module '@rjsf/core' {
}

declare module '@rjsf/core/lib/components/fields/SchemaField' {
import { JSONSchema7 } from 'json-schema';
import { FieldProps, UiSchema, IdSchema, FormValidation } from '@rjsf/core';
import { FieldProps } from '@rjsf/core';

export type SchemaFieldProps<T = any> = Pick<
FieldProps<T>,
Expand Down
25 changes: 18 additions & 7 deletions packages/core/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 4 additions & 2 deletions packages/core/package.json
Expand Up @@ -15,9 +15,10 @@
"publish-to-npm": "npm run build && npm publish",
"start": "concurrently \"npm:build:* -- --watch\"",
"tdd": "cross-env NODE_ENV=test mocha --require @babel/register --watch --require ./test/setup-jsdom.js test/**/*_test.js",
"test": "cross-env BABEL_ENV=test NODE_ENV=test mocha --require @babel/register --require ./test/setup-jsdom.js test/**/*_test.js",
"test": "concurrently \"cross-env BABEL_ENV=test NODE_ENV=test mocha --require @babel/register --require ./test/setup-jsdom.js test/**/*_test.js\" \"npm:test-type\"",
"test-coverage": "cross-env NODE_ENV=test nyc --reporter=lcov mocha --require @babel/register --require ./test/setup-jsdom.js test/**/*_test.js",
"test-debug": "cross-env NODE_ENV=test mocha --require @babel/register --require ./test/setup-jsdom.js --debug-brk --inspect test/Form_test.js"
"test-debug": "cross-env NODE_ENV=test mocha --require @babel/register --require ./test/setup-jsdom.js --debug-brk --inspect test/Form_test.js",
"test-type": "tsc"
},
"lint-staged": {
"{src,test}/**/*.js": [
Expand Down Expand Up @@ -100,6 +101,7 @@
"rimraf": "^2.5.4",
"sinon": "^9.0.2",
"style-loader": "^0.13.1",
"typescript": "^4.2.3",
"webpack": "^4.42.1",
"webpack-cli": "^3.1.2",
"webpack-dev-middleware": "^3.4.0",
Expand Down
25 changes: 25 additions & 0 deletions packages/core/tsconfig.json
@@ -0,0 +1,25 @@
{
"compilerOptions": {
"module": "commonjs",
"lib": [
"es6",
"dom"
],
"noImplicitAny": true,
"noImplicitThis": true,
"strictNullChecks": true,
"strictFunctionTypes": true,
"baseUrl": "../",
"typeRoots": [
"../"
],
"types": [],
"noEmit": true,
"forceConsistentCasingInFileNames": true,
"jsx": "react"
},
"files": [
"index.d.ts",
"typing_test.tsx"
]
}