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

styled(component)(``) API not detected #30

Closed
oliviertassinari opened this issue May 26, 2024 · 7 comments
Closed

styled(component)(``) API not detected #30

oliviertassinari opened this issue May 26, 2024 · 7 comments

Comments

@oliviertassinari
Copy link

oliviertassinari commented May 26, 2024

if (ts.isTaggedTemplateExpression(node) && isStyledComponent(node)) {

is not enough to detect APIs like https://codesandbox.io/p/sandbox/styled-components-forked-v3yv3p?file=%2FTitle.js%3A10%2C1

import styled from "@emotion/styled";

// Create a <Title> react component that
// renders an <h1> which is centered, palevioletred and sized at 1.5em
export default styled("h1")(`
  font-size: 1.5em;
  text-align: center;
  color: palevioletred;
`);

but it works, should we fix this?

To be clear styled(Component)`` works but styled(Component)(``) doesn't.

@hudochenkov
Copy link
Owner

It doesn't seem to be supported by Styled Components: https://codesandbox.io/p/sandbox/styled-components-base-forked-m4n2fg?file=%2FTitle.js%3A16%2C1

Is it something that Pigment CSS or Emotion support?

@oliviertassinari
Copy link
Author

oliviertassinari commented May 27, 2024

@hudochenkov Ah, you are right, I jumped too quickly to a conclusion. It's Emotion that supports it: https://codesandbox.io/p/sandbox/styled-components-base-forked-2sxd28?file=%2FTitle.tsx%3A31%2C1

SCR-20240527-lpgl

Pigment CSS supports it technically, but will throw TypeScript errors. I guess this makes the most sense, to have a single way to do things, so throw TypeScript to encourage developers to standardize. cc @brijeshb42


@mui/system supports this API like Emotion, but it feels like it should be standardized to behave like Pigment CSS.

@hudochenkov
Copy link
Owner

I feel that styled("h1")(`...`); is working in Emotion by chance. It is not documented, and I couldn't find in Emotion codebase any tests for this. I was searching for (` and (\`.

I think postcss-styled-syntax shouldn't have support for this, since it's not something should have ever worked in the first place :)

@oliviertassinari
Copy link
Author

oliviertassinari commented May 28, 2024

I also got tricked by https://github.com/styled-components/vscode-styled-components which highlights it.

SCR-20240528-ohkm

Issue created: styled-components/vscode-styled-components#446

I feel that styled("h1")(...); is working in Emotion by chance.

Seems accurate https://github.com/emotion-js/emotion/blob/main/packages/styled/__tests__/styled.js.

Ok, I'm happy to standardize on this. It makes sense to me to not make it possible to do things the same way. I'm removing these instances from the codebase of MUI.

@hudochenkov
Copy link
Owner

Closing as we reached a consensus.

@oliviertassinari
Copy link
Author

Hopefully, the VS Code plugin extension will agree 😄 styled-components/vscode-styled-components#446

@hudochenkov
Copy link
Owner

If actual styled-components doesn't support that code, there is no point of linting it, or syntax highlight. Unless we want to support Emotion's undocumented feature :)

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

No branches or pull requests

2 participants