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

fix 3759 - uneditable & permanent defaults with additional properties #4440

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

Piotr-Debicki
Copy link

@Piotr-Debicki Piotr-Debicki commented Jan 8, 2025

Reasons for making this change

Fixes #3759

Additional properties with defaults set currently creates a permanent key-value pair that is essentially uneditable.
Trying to edit the key creates an entirely new key value entry on blur while still keeping the default key-value pair. The default key-value pair is also not deletable. Solution here generates defaults for additional properties once on form initialization, and then skips regeneration on any form update

Something to keep in mind with this solution is that defaults for additional properties will not generate if custom formData is provided to the form - a way around that would be to keep track of something like a defaults generated state instead of relying on 'edit'

Checklist

  • I'm updating documentation
  • I'm adding or updating code
    • I've added and/or updated tests. I've run npx nx run-many --target=build --exclude=@rjsf/docs && npm run test:update to update snapshots, if needed.
    • I've updated docs if needed
    • I've updated the changelog with a description of the PR
  • I'm adding a new feature
    • I've updated the playground with an example use of the feature

@heath-freenome
Copy link
Member

heath-freenome commented Jan 10, 2025

@Piotr-Debicki what happens if you add additional properties to an object nested within another object instead? Do your changes fix this? If not, you may need to improve the getDefaultFormState() function instead.

schema = {
  type: 'object',
  nested: {
  properties: {
    baz: {
      type: 'string';
    },
    blah: {
      type: 'object',
      additionalProperties: {
        type: 'string',
      },
      default: {
        foo: 'bar',
      },
    }
  }
};

Rollback previous change
@Piotr-Debicki
Copy link
Author

Piotr-Debicki commented Jan 10, 2025

@Piotr-Debicki what happens if you add additional properties to an object nested within another object instead? Do your changes fix this? If not, you may need to improve the getDefaultFormState() function instead.

schema = {
  type: 'object',
  nested: {
  properties: {
    baz: {
      type: 'string';
    },
    blah: {
      type: 'object',
      additionalProperties: {
        type: 'string',
      },
      default: {
        foo: 'bar',
      },
    }
  }
};

@heath-freenome From what I can tell these seem to be two separate issues. The issue this solution fixes is just the regeneration of defaults for additional properties on form change, (e.g. pressing the delete button on the default key will just regenerate the default key since it 'changed', making it a permanent fixture) - not the generation of the defaults themselves or how they're generated. I can take a look at the bug you presented when I get a bit more time, but these changes didn't have that specific bug in mind

CHANGELOG.md Outdated Show resolved Hide resolved
Co-authored-by: Heath C <[email protected]>
@heath-freenome
Copy link
Member

@Piotr-Debicki The main reason why I suggested the above test was that, I believe, the code fix belongs in the getDefaultFormState() function rather than in Form since the former deals with the recursion deeper into the schema (and would handle the outermost case as well). Does that make sense?

@Piotr-Debicki
Copy link
Author

@heath-freenome Gotcha, I'll take another look at it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants