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

Macro system does not support calling a function which calls styled #24

Closed
ncknuna opened this issue Jan 4, 2023 · 6 comments
Closed

Comments

@ncknuna
Copy link

ncknuna commented Jan 4, 2023

[transferred to an issue from the vanilla extract discord]

Hello! I'm thinking of moving a codebase from emotionjs to macaron, but I've run into a sticking point: our component library often wraps a third-party component lib (ant design in this case) and we often need to restyle the underlying component lib in certain situations.

In emotion, we could select the classnames that the third-party library was exposing, so we could do something like this:
// styles for my current component
paddingLeft: 12,
// styles for the third party lib
'& .ant-table-thead': {
fontSize: 24
},
'& .ant-table-tbody': ...
// other styles

Since vanilla extract doesn't support complex selectors targeting elements other than the current one, the easiest way I could think of doing this with vanilla extract is something like this:
const wrapperCls = style({
paddingLeft: 12,
})
globalStyle(${wrapperCls} > .ant-table-thead, {
fontSize: 24
})
globalStyle(${wrapperCls} > .ant-table-tbody, ...)
// other calls to globalStyle

Which, while technically maybe the same number of lines, feels more clunky and forces a separation that I think makes the intention of the code less clear. I thought about possibly writing a utility function that would take the object style and make those multiple globalStyle calls for me, but it seems to throw "Maximum call stack size exceeded" errors, because I think the macro system doesn't know how to resolve the variable references.

Here's a stackblitz link: https://stackblitz.com/edit/macaron-css-macaron-rjwpfg?file=src/App.tsx

I was able to get it to work with Vanilla Extract (without macaron) if that's helpful: https://stackblitz.com/edit/vitejs-vite-63vac7?file=src/components/button/button.css.ts . Macaron would offer a cleaner upgrade path for me and closer to the DX that I'm looking for though

@Mokshit06 replied:
oh so i see what's happening, macaron is trying to extract the styled function inside sampleFn but it depends on a variable reference that is declared inside it so it is unable to extract

we should move this to a github issue

@Mokshit06
Copy link
Member

Mokshit06 commented Jan 4, 2023

@ncknuna This should be fixed in release 1.1.3. The issue was that macaron tries to extract every call of styled functions, but it was unable to do so because of a variable reference that it couldn't resolve. In this case you want to opt out of macaron's extraction since you know you will only call it inside macaron$ and it will be extracted later. So you can add a leading comment to the function calls with macaron-ignore like

const Component = /* macaron-ignore */ styled("button", {});
/* macaron-ignore */ globalStyle(selector, styles)

macaron will skip extracting these functions and will directly extract where you've called macaron$. Just add these comments inside sampleFn and it should work

@Mokshit06
Copy link
Member

Another thing, you can't return a styled component from macaron$() because the return value needs to be serialised and component declarations are functions and can't be serialised.

@ncknuna
Copy link
Author

ncknuna commented Jan 5, 2023

Thanks for the quick fix! I forked and modified the previous stackblitz and now it works: https://stackblitz.com/edit/macaron-css-macaron-tj3yss?file=src/App.tsx

The main concern I have now is that it's a bit verbose, needing to be wrapped by both macaron$ and sampleFn:

const MyComponent = getStyledPrimativeComponent(
  'div',
  macaron$(() =>
    sampleFn({
      fontSize: 24,
      backgroundColor: 'green',
      '& .inner': {
        backgroundColor: 'blue',
      },
    })
  )
);

Can you think of any way to make this a bit more elegant? Also happy to take this to a discussion now that the original issue was fixed, and close this one.

@Mokshit06
Copy link
Member

I have a few ideas on how this can be made less verbose. Lets move this to a discussion

@Mokshit06
Copy link
Member

Vanilla-extract also has a concept of function serialisers that it uses to serialise recipes etc, which allow them to be returned from macaron$. Maybe styled can also be made into a serialisable function, so you can just return it directly

@ncknuna
Copy link
Author

ncknuna commented Jan 9, 2023

Started disccusion here: #26

Closing this as completed

@ncknuna ncknuna closed this as completed Jan 9, 2023
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