-
Notifications
You must be signed in to change notification settings - Fork 28
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
Add Guessers based on the OpenAPI Schema #80
Conversation
supabaseClient: (url: string, options?: any) => | ||
httpClient(`${config.apiUrl}/${url}`, options), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where is this used?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is not, but I initially wanted to add the getSchema method directly in the demo app, only to realize I was missing the ability to call the API. This will prove very useful for custom endpoints.
packages/ra-supabase-ui-materialui/src/guessers/ShowGuesser.tsx
Outdated
Show resolved
Hide resolved
packages/ra-supabase-ui-materialui/src/guessers/ShowGuesser.tsx
Outdated
Show resolved
Hide resolved
packages/ra-supabase-ui-materialui/src/guessers/ShowGuesser.tsx
Outdated
Show resolved
Hide resolved
packages/ra-supabase-ui-materialui/src/guessers/ShowGuesser.tsx
Outdated
Show resolved
Hide resolved
React.useEffect(() => { | ||
if (isPending || error) { | ||
return; | ||
} | ||
const resourceDefinition = schema.definitions?.[resource]; | ||
const requiredFields = resourceDefinition?.required || []; | ||
if (!resourceDefinition || !resourceDefinition.properties) { | ||
throw new Error( | ||
`The resource ${resource} is not defined in the API schema` | ||
); | ||
} | ||
if (!resourceDefinition || !resourceDefinition.properties) { | ||
return; | ||
} | ||
const inferredInputs = Object.keys(resourceDefinition.properties) | ||
.filter((source: string) => source !== 'id') | ||
.map((source: string) => | ||
inferElementFromType({ | ||
name: source, | ||
types: editFieldTypes, | ||
description: | ||
resourceDefinition.properties![source].description, | ||
format: resourceDefinition.properties![source].format, | ||
type: (resourceDefinition.properties && | ||
resourceDefinition.properties[source] && | ||
typeof resourceDefinition.properties[source].type === | ||
'string' | ||
? resourceDefinition.properties![source].type | ||
: 'string') as string, | ||
requiredFields, | ||
}) | ||
); | ||
const inferredForm = new InferredElement( | ||
editFieldTypes.form, | ||
null, | ||
inferredInputs | ||
); | ||
setChild(inferredForm.getElement()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should probably be a hook reusable between views
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I disagree. This refactoring is not straightforward as we can only mutualize part of the effect (the console.log is different), and it has to interface with a state defined outside. I assume the refactoring you suggest will produce code that is harder to read (and the current code is already hard to read).
packages/ra-supabase-ui-materialui/src/guessers/ShowGuesser.tsx
Outdated
Show resolved
Hide resolved
const resourceDefinition = schema.definitions?.[resource]; | ||
const requiredFields = resourceDefinition?.required || []; | ||
if (!resourceDefinition || !resourceDefinition.properties) { | ||
throw new Error( | ||
`The resource ${resource} is not defined in the API schema` | ||
); | ||
} | ||
if (!resourceDefinition || !resourceDefinition.properties) { | ||
return; | ||
} | ||
const inferredInputs = Object.keys(resourceDefinition.properties) | ||
.filter((source: string) => source !== 'id') | ||
.filter( | ||
source => | ||
resourceDefinition.properties![source].format !== 'tsvector' | ||
) | ||
.map((source: string) => | ||
inferElementFromType({ | ||
name: source, | ||
types: editFieldTypes, | ||
description: | ||
resourceDefinition.properties![source].description, | ||
format: resourceDefinition.properties![source].format, | ||
type: (resourceDefinition.properties && | ||
resourceDefinition.properties[source] && | ||
typeof resourceDefinition.properties[source].type === | ||
'string' | ||
? resourceDefinition.properties![source].type | ||
: 'string') as string, | ||
requiredFields, | ||
}) | ||
); | ||
const inferredForm = new InferredElement( | ||
editFieldTypes.form, | ||
null, | ||
inferredInputs | ||
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry to insist but this could be extracted at least as a getInferredView
function taking the following parameters:
types
: here it would beeditFieldTypes
,listFieldTypes
inListGuesser
containerType
: hereeditFieldTypes.form
,listFieldTypes.table
inListGuesser
That would make it a lot easier to read and avoid a lot of duplication. It could also be a hook but that may hurt comprehension of the code
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code is different in each guesser. Sometimes we include requiredFields, and sometimes not. Sometimes we consider id
fields, and sometimes not. Plus, the names of the variables are more readable (inferredFoem
, inferredInput
) than any abstract counterpart.
packages/ra-supabase-ui-materialui/src/guessers/CreateGuesser.tsx
Outdated
Show resolved
Hide resolved
packages/ra-supabase-ui-materialui/src/guessers/ListGuesser.tsx
Outdated
Show resolved
Hide resolved
<Resource name="${def.name}"${ | ||
def.list ? ' list={ListGuesser}' : '' | ||
}${def.edit ? ' edit={EditGuesser}' : ''}${ | ||
def.create ? ' create={CreateGuesser}' : '' | ||
}${def.show ? ' show={ShowGuesser}' : ''} />` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
question: Contact and Sale default recordRepresentation
are wrong. Do you think it would be possible to use the OpenAPI schema to fix them?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think the OpenAPI spec allows for that. I've seen nothing related in the default PostgREST OpenAPI output, nor in the PostgREST OpenAPI doc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💪
Problem
React-admin's guessers fetch data and inspect it to scaffold CRUD views for a resource.
But there is no
CreateGuesser
, and the guessers sometimes make wrong assumptions.Solution
Supabase uses PostgREST, which exposes an OpenAPI schema.
Provide alternative guessers that read this schema instead of fetching data.
Zero-config admin:
Guessed admin:
Guessed Company list:
To do
dataProvider.getSchema()
for schema introspectionuseAPISchema
hook to access is from React componentsListGuesser
leveraging OpenAPI, and guessing filters, tooShowGuesser
leveraging OpenAPIEditGuesser
leveraging OpenAPICreateGuesser
leveraging OpenAPIAdminGuesser
leveraging OpenAPI