macaron-css / macaron

Compiler-augmented typesafe CSS-in-JS with zero runtime, colocation, maximum safety and productivity
https://macaron.js.org/
MIT License
735 stars 16 forks source link

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

Closed ncknuna closed 1 year ago

ncknuna commented 1 year ago

[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 commented 1 year ago

@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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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

Mokshit06 commented 1 year ago

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 commented 1 year ago

Started disccusion here: https://github.com/macaron-css/macaron/discussions/26

Closing this as completed