styled-components / xstyled

A utility-first CSS-in-JS framework built for React. 💅👩‍🎤⚡️
https://xstyled.dev
MIT License
2.28k stars 105 forks source link

feat: styled.extend #250

Closed agriffis closed 3 years ago

agriffis commented 3 years ago

Summary

For proper pluggability, we need styled.extend to complement the existing x.extend. See #246

This is not the API I would like eventually for a unified xstyled creator, but it's still worthwhile and a step in the right direction.

API details

Add createStyled (like createX) so that styled.extend can call it.

The recently-added styledWithGenerator becomes styled.withGenerator for styled-components, and styled(..., {generator}) for emotion, in each case adhering to their existing API without changing function arities.

Status

This is a work in progress...

agriffis commented 3 years ago

Interesting gotcha, we can extend styled one way but not the other, at least not without additional work.

We can do this:

const borderInlineWidth = style({
  prop: ['biw', 'borderInlineWidth'],
  css: 'borderInlineWidth',
  themeGet: getSpace,
})

const myStyled = styled.extend(borderInlineWidth)

const Dummy = myStyled``

<Dummy biw={1} /> // border-inline-width: 4px; ✅

but we can't do this:

const Dummy = myStyled`
  border-inline-width: 1;
`

<Dummy /> // border-inline-width: 1; ❌

The reason is that transform() refers to the statically-defined propGetters.

I added a skipped test for this, for now. To fix it properly, we would need createPropGetters and createTransform which makes sense in a fully-extensible xstyled, but it's more work than I'd anticipated at this juncture. I may consider that in a separate pull request to keep this one from getting out of hand.

agriffis commented 3 years ago

This is otherwise coming together nicely. I just need to review a couple types that got lost along the way.

agriffis commented 3 years ago

Tests run clean, but build fails with various TS errors.

✸ yarn build
...
lerna ERR! yarn run build exited 1 in '@xstyled/emotion'
lerna ERR! yarn run build stderr:

src/index.ts → dist/index.js, dist/index.mjs...
(!) Mixing named and default exports
https://rollupjs.org/guide/en/#outputexports
The following entry modules are using named and default exports together:
src/index.ts

Consumers of your bundle will have to use chunk['default'] to access their default export, which may not be what you want. Use `output.exports: 'named'` to disable this warning
created dist/index.js, dist/index.mjs in 106ms

src/index.ts → dist/index.d.ts...
src/styled.ts(4,14): error TS4023: Exported variable 'styled' has or is using name 'CreateXStyled' from external module "/home/aron/src/xstyled/packages/emotion/src/createStyled" but cannot be named.
...

It's not clear to me how I'm using named and default exports together in any way that's different from before.

and the second error seems to be referencing Voldemort.

agriffis commented 3 years ago

@gregberge Agreed. The more I work on this, the more it needs to be holistic instead of piecemeal.

Also I need to learn TS better, but it's taking longer than I'd anticipated.

gregberge commented 3 years ago

Closed in favor of #262