dnbexperience / eufemia

DNB Design System
https://eufemia.dnb.no
Other
53 stars 32 forks source link

Forms: Provide a way of resetting or scoping form data stored by id? #3803

Closed kimroen closed 3 months ago

kimroen commented 3 months ago

🐛 Bug Report

When a form has been defined with an id, the contents of the form persists even when react re-mounts the application. This sounds like it might be the intention, so I don't necessarily want to call it a bug, but wanted to raise it since it causes problems for us.

We have a form with an id that we render in a test. When we run multiple tests, the state of the form persists from the first test to the next.

To Reproduce

Make a test file with this content (sorry, I couldn't make testing-library run properly in codesandbox):

import "@testing-library/jest-dom/extend-expect";
import React from "react";
import { render, screen } from "@testing-library/react";
import user from "@testing-library/user-event";
import { Form, Field } from "@dnb/eufemia/extensions/forms";

function MyForm() {
  return (
    <Form.Handler
      id="my-id"
      defaultData={{
        firstName: "",
      }}
    >
      <Field.String path="/firstName" label="First name" />
    </Form.Handler>
  );
}

test("Filling out the form", async () => {
  render(<MyForm />);

  const nameField = screen.getByRole("textbox", { name: /first name/i });
  await userEvent.type(nameField, "Sally");

  expect(nameField).toHaveValue("Sally");
});

test("Filling out the form again", async () => {
  render(<MyForm />);

  const nameField = screen.getByRole("textbox", { name: /first name/i });
  await userEvent.type(nameField, "John");

  expect(nameField).toHaveValue("John");
});

The second test fails, because the value of the field is "SallyJohn", and not "John".

Expected behavior

The form does not retain the entered data between tests. Either by some way of resetting the state between tests, or by providing a way of scoping it to a context (perhaps the EufemiaProvider).

Eufemia Version

Node.js: 10.43.0

kimroen commented 3 months ago

I believe this is handled in this variable in module scope, so resetting module state between each test could be one way of getting around this. This would slow down the test suite a lot though.

tujoworker commented 3 months ago

Thank you for your good report/feature request.

How about the API:

import { Tools, Form } from '@dnb/eufemia/extensions/forms'

beforeEach(() => {
  Tools.formReset('unique-id')
})

What would you recommend, like:

tujoworker commented 3 months ago

In a meeting the suggestion goes towards: Form.clearData

kimroen commented 3 months ago

Sounds good to me! The "reset" wording kind of implies that it'll go back to the default values, but it's probably not going to do that, right? You would need to render the form again to get the default values back in there.

Also agreed on keeping it on Form instead of ~introducing Tools~ I saw afterwards that we already have Tools, but I still agree 😊

tujoworker commented 2 months ago

:tada: This issue has been resolved in version 10.44.0 :tada:

The release is available on:

Your semantic-release bot :package::rocket: