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

support for fragments #38

Open
davidmarkclements opened this issue Jul 25, 2019 · 1 comment
Open

support for fragments #38

davidmarkclements opened this issue Jul 25, 2019 · 1 comment

Comments

@davidmarkclements
Copy link

The following will fail with error:

TypeError: Cannot convert a Symbol value to a string
const cmp = () =>  {
  return (<Style>
    {styles}
    <>
    <label htmlFor={name} className="label">{label}</label>
    <input ref={ref} id={name} name={name} className="text-input"/>
    <button onClick={onButtonClick} className="button">{buttonText}</button>
    </>
  </Style>)
}

The error comes from this line https://github.com/buildbreakdo/style-it/blob/master/src/index.js#L303 and it's because React.Fragment is a symbol

I realise the approach for this may have to deviate from the current approach (e.g. it'll probably require prefixing the generated scope class string to all classes on elements within a fragment) - but it would also be handy if it "Just Worked" and fits well with the philosophy of optimizing for user consumption.

Also this may or may not relate to #25

@buildbreakdo
Copy link
Owner

Hi David, thanks for opening this. Agree fragment support should be implemented. Fragments had yet to be added to React when style-it was originally created. Does add some complexity in terms of the scoping class, for a fragment with many children, each element basically becomes a root node, so a scoping class would have to be attached for each. Which means calling cloneElement N times instead of one time (to add the scoping class).

It's likely the solution is to not use clone element at all, this is preferable from a computation perspective and requires a bit of a re-architecture. Believe the next iteration of this project is to use the style element itself as the root selector in tandem with the next sibling selector + to scope styles. Output would look something like this:

<style id="scope-1234">
  #scope-1234 + div {
    color: red;
  }
</style>
<div>
  <span>abcdefg</span>
<div>

Or with your example output would be:

<style id="scope-1234">
  #scope-1234 + label {
    color: red;
  }
  #scope-1234 + label + input {
    color: red;
  }
  #scope-1234 + label + input + button {
    color: red;
  }
</style>

<label htmlFor={name} className="label">{label}</label>
<input ref={ref} id={name} name={name} className="text-input"/>
<button onClick={onButtonClick} className="button">{buttonText}</button>

This would require knowing the root node type of the fragment children to build out the sibling traversal selectors. This would avoid expensive calls to cloneElement (which would like to move completely away from).

Some thoughts to think about, love to hear yours. Thanks again for bringing this up!

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