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

Should formatIs ranker support null types? #1804

Closed
alastair-todd opened this issue Sep 13, 2021 · 4 comments
Closed

Should formatIs ranker support null types? #1804

alastair-todd opened this issue Sep 13, 2021 · 4 comments

Comments

@alastair-todd
Copy link

Describe the bug

export const formatIs = (expectedFormat: string): Tester =>
  schemaMatches(
    schema =>
      !isEmpty(schema) &&
      schema.format === expectedFormat &&
      schema.type === 'string'
  );

Expected behavior

Should support nulls

type: ['string', 'null']

Steps to reproduce the issue

N/A

Screenshots

No response

In which browser are you experiencing the issue?

N/A

Framework

Core

RendererSet

No response

Additional context

No response

@sdirix
Copy link
Member

sdirix commented Sep 13, 2021

Hi @alastair-todd thanks for the report. If I read the spec correctly we should probably not hard code a check against null but rather check whether string is included in the type array.

The only problem I see here is for users who are already using formatIs, rely on the type always being a string and then suddenly their custom renderer is also invoked on other types.

@sdirix sdirix added this to the next milestone Sep 13, 2021
@sdirix
Copy link
Member

sdirix commented Sep 13, 2021

In the mean time you can always write your own version of formatIs which works "properly"

@alastair-todd
Copy link
Author

alastair-todd commented Sep 13, 2021

Well yeah I've gone and created my own testers.

const formatIs = (expectedFormat: string): Tester => 
  schemaMatches((schema) => !isEmpty(schema) && schema.format === expectedFormat);

Its the same for oneOf with a null value e.g.

[
   {
      "title":" ",
      "type":"null"
   },
   {
      "title":"Permanent",
      "const":"Permanent"
   },
   {
      "title":"Temporary",
      "const":"Temporary"
   },
   {
      "title":"Casual",
      "const":"Casual"
   }
]
const isOneOfEnumControl = and(
  uiTypeIs('Control'),
  schemaMatches(schema =>
    schema.hasOwnProperty('oneOf')
  )
);

(removed the in-built version's forcing all options to be const)

@sdirix
Copy link
Member

sdirix commented Nov 25, 2022

This was fixed with #1925

@sdirix sdirix closed this as completed Nov 25, 2022
@sdirix sdirix removed this from the next milestone Nov 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants