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

AutoField.componentDetectorContext is bypassed if schema specify a component #1114

Closed
macrozone opened this issue May 16, 2022 · 4 comments · Fixed by #1179
Closed

AutoField.componentDetectorContext is bypassed if schema specify a component #1114

macrozone opened this issue May 16, 2022 · 4 comments · Fixed by #1179
Assignees
Labels
Area: Core Affects the uniforms package Type: Feature New features and feature requests
Milestone

Comments

@macrozone
Copy link
Contributor

macrozone commented May 16, 2022

in ReactPage a lot of users asked for having conditional fields in the schema, so by default we support an additional prop showIf that can be used in the schema to control whether a field should be shown based on other props. The implementation was fairly easy:

import React from 'react';
import { AutoField } from 'uniforms-material';

const AutofieldContextProvider: React.FC = ({ children }) => (
  <AutoField.componentDetectorContext.Provider
    value={(props, uniforms) => {
      const show = props.showIf?.(uniforms.model) ?? true;
      return show
        ? AutoField.defaultComponentDetector(props, uniforms)
        : () => null;
    }}
  >
    {children}
  </AutoField.componentDetectorContext.Provider>
);

export default AutofieldContextProvider;

unfortunalty a user noticed that this does not work when you specify a custom component in the schema using uniforms.component prop.

the provider above is never called for these fields.

I think it makes sense at first glance that any custom detector is bypassed when a component is specified, but still, it removes a lot of flexibility like the case above. Since autoField gets removed (#980), I actually ran out of ideas how to implement the showIf usecase properly.

the problematic code line is this: https://github.com/vazco/uniforms/blob/master/packages/uniforms/src/createAutoField.tsx#L35

it would probably better if the componentDetector is responsible of handling the case where component is defined on the field

@macrozone
Copy link
Contributor Author

changing that is probably a breaking change, maybe there is a way to opt-in that the componentDetector is always called? Some flag?

@macrozone
Copy link
Contributor Author

maybe that could be a property of the bridge?

componentDetectorMode: "ifNotSet" | "always"

ifNotSet: calls the dector only if field does not specify component (default behavior like now)
always: always calls the detector, detector has to include handling of specifying a component

@radekmie radekmie self-assigned this May 20, 2022
@radekmie radekmie added the Type: Feature New features and feature requests label May 20, 2022
@radekmie radekmie added this to the v4.0 milestone May 20, 2022
@radekmie
Copy link
Contributor

Hey @macrozone. Indeed, making AutoField always call the componentDetector sounds like a good idea. However, it is a breaking change, so we'll probably have to postpone it to v4. It's easily solvable and it won't break anything, so maybe... I have to think about it.

@radekmie radekmie added the Area: Core Affects the uniforms package label Jun 10, 2022
@Monteth
Copy link
Member

Monteth commented Sep 2, 2022

A summary from the v4 planning session for this ticket is we will move the props.component to the defaultComponentDetector.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: Core Affects the uniforms package Type: Feature New features and feature requests
Projects
Archived in project
4 participants