-
-
Notifications
You must be signed in to change notification settings - Fork 638
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
Configuring plugin to work with polymorphic "as" prop components #923
Comments
It's unfortunate after decades of folks realizing polymorphism is something to be avoided, that polymorphic patterns are still being used - but that said, it seems like a problem worth solving if people are insisting on doing it anyway. What schema would you suggest? |
@ljharb I'm keen to learn a better way besides polymorphic components to address the use case of easily passing style-related props to any html element, if you're aware of better practice. For example:
<Box width={[1, 1 / 2]} p={4} mb={3} bg="tomato">
This is a Styled System box
</Box>
<Box bg='tomato' w='100%' p={4} color='white'>
This is a Chakra UIBox
</Box>
<Text as="h1" intent="body">This looks like regular text but is a h1 element</Text>
<Text as="p" intent="title">This looks like a large title but is a p element</Text>
<Text as="button" intent="subheading" onClick={handleClick}>This looks like a subheading title but is a p but is a button</Text> |
In my experience each thing has a manually created wrapper component for it, with explicit props to control the appearance - not actually allowing anyone to pass specific colors or sizes etc, but instead, providing an explicit list of named configurations. That said, let's keep the issue on topic :-) what schema do you suggest for a component that has a specific property to govern its "kind"? |
Thanks. If what you mean by schema is the JSON schema for amending the plugin, how's this:
The biggest limitation which I've not good solution for is how does the linter work with conditional
Would it be possible for the linter to handle this if we passed the appropriate html attributes conditionally? E.g. const App = () => {
const isClickable = true;
return (
<Box
...{isClickable ? {
as: "button",
onClick: () => { console.log("clicked") },
...otherButtonProps
} : {
as: "div"
}
>
I may be a clickable button. How do I lint this?
</Box>
)
} If it's the implementation detail, I'm probably going to have to take a deeper look into the a11y plugin. |
Wanted to jump in and +1, this would be great!! This also came up in: #863.
I agree. This might be a bit more complex to figure out, and I think it's worth kicking off support with just literal mapping and iterate on that. I believe we could get something going pretty quickly with just updating: getElementType. |
I don't have super strong opinions, but just some initial thoughts/questions on the proposed schema:
Should this linter care about if something maps to a React fragment? Based on the existing rules, it seems like the linter only cares if something maps to an HTML element it recognizes, so maybe for now, we don't need to worry about explicitly mapping fragments in the configs.
Given this is a "special value", could it be more appropriate as an explicit key?
I am assuming that if this linter comes across something like:
I like that this makes it convenient for the consumer to set multiple components to the same config as I'd imagine a lot of components will have the same exact config, but I also find it a bit less-readable than if we used the component as a key like in |
Having trouble applying this plugin to polymorphic components that take an
as
orcomponent
prop which determines the html element the component will be rendered as.For instance the commonly used
Box
component pattern like the one in Material UI:The WordPress Gutenberg team seem to have the same issue.
The text was updated successfully, but these errors were encountered: