feat: styled.extend #250

agriffis · 2021-04-26T12:55:12Z

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...

styled-components rough pass
emotion rough pass
tests for styled.extend
types cleanup

agriffis · 2021-05-04T16:27:18Z

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 · 2021-05-04T16:30:05Z

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

I misunderstood how xstyled's css function interacts with emotion's css function, so I added this complicated code in c400c70. More recently, while working on styled-components#250, I discovered that it's also broken, because it converts a function argument into an array. This simpler code does the one thing it needs to do: add string separators for the tagged template literal for the number of generators. It also produces a slightly more efficient result, because it tests `generators.length` before making the created function, rather than testing it every time the function runs.

I misunderstood how xstyled's css function interacts with emotion's css function, so I added this complicated code in c400c70. More recently, while working on styled-components#250, I discovered that it's also broken, because it converts a function argument into an array. This simpler code does the one thing it needs to do: add string separators for the tagged template literal for the number of generators. It also produces a slightly more efficient result, because it does more work up front (testing `generators.length` and concatting arrays) before the resulting function, rather than every time the function runs.

I misunderstood how xstyled's css function interacts with emotion's css function, so I added this complicated code in c400c70. More recently, while working on #250, I discovered that it's also broken, because it converts a function argument into an array. This simpler code does the one thing it needs to do: add string separators for the tagged template literal for the number of generators. It also produces a slightly more efficient result, because it does more work up front (testing `generators.length` and concatting arrays) before the resulting function, rather than every time the function runs.

This is a relatively pure refactor without new functionality, in preparation for adding `styled.extend()` First, add `createStyled` to emit `styled` given a generator function. Second, replace `styledWithGenerator` by `styled.*.withGenerator` (styled-components) and `styled(*, { generator })` (emotion). This avoids the creator needing to emit more than one function with different capabitilies.

This works similarly to `x.extend()`. The styled component will respond to the props supported by the supplied generator(s). However it doesn't do extra replacments on the CSS template string, because that would additionally require `createPropGetters` and `createTransform`.

agriffis · 2021-05-04T22:03:09Z

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.

gregberge

It is a good start but I think we should not release it until we have the good API for that. It will be probably for a v3.1.

agriffis · 2021-05-09T13:09:13Z

@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 · 2021-05-22T15:52:45Z

Closed in favor of #262

I misunderstood how xstyled's css function interacts with emotion's css function, so I added this complicated code in c400c70. More recently, while working on #250, I discovered that it's also broken, because it converts a function argument into an array. This simpler code does the one thing it needs to do: add string separators for the tagged template literal for the number of generators. It also produces a slightly more efficient result, because it does more work up front (testing `generators.length` and concatting arrays) before the resulting function, rather than every time the function runs.

agriffis force-pushed the feat/styled-extend branch from fba9dec to 3ebdcd7 Compare April 28, 2021 01:28

gregberge force-pushed the next branch from 115b78d to 6253b8f Compare May 2, 2021 21:06

agriffis force-pushed the feat/styled-extend branch 2 times, most recently from de6ddbc to 65fe5f6 Compare May 4, 2021 16:18

agriffis mentioned this pull request May 4, 2021

fix(emotion): broken getCreateStyle #254

Merged

agriffis force-pushed the feat/styled-extend branch from 65fe5f6 to 196dbb0 Compare May 4, 2021 21:25

agriffis added 2 commits May 4, 2021 17:59

agriffis force-pushed the feat/styled-extend branch from 196dbb0 to 8c90dba Compare May 4, 2021 21:59

gregberge approved these changes May 9, 2021

View reviewed changes

gregberge closed this May 22, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: styled.extend #250

feat: styled.extend #250

agriffis commented Apr 26, 2021 •

edited

Loading

agriffis commented May 4, 2021 •

edited

Loading

agriffis commented May 4, 2021

agriffis commented May 4, 2021 •

edited

Loading

gregberge left a comment

agriffis commented May 9, 2021

gregberge commented May 22, 2021

feat: styled.extend #250

feat: styled.extend #250

Conversation

agriffis commented Apr 26, 2021 • edited Loading

Summary

API details

Status

agriffis commented May 4, 2021 • edited Loading

agriffis commented May 4, 2021

agriffis commented May 4, 2021 • edited Loading

gregberge left a comment

Choose a reason for hiding this comment

agriffis commented May 9, 2021

gregberge commented May 22, 2021

agriffis commented Apr 26, 2021 •

edited

Loading

agriffis commented May 4, 2021 •

edited

Loading

agriffis commented May 4, 2021 •

edited

Loading